Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include CSV header in data files #220

Merged
merged 1 commit into from
Jul 12, 2023
Merged

Conversation

naomiGrew
Copy link
Contributor

@naomiGrew naomiGrew commented Jul 10, 2023

Resolves #188

@smarr
Copy link
Owner

smarr commented Jul 10, 2023

Could you add a test for this in https://github.com/smarr/ReBench/blob/master/rebench/tests/persistency_test.py?

It has some examples. Currently, it doesn't really access the file directly to check that the data is in there, but only loads the data.

For our purposes, it would be good read the file just directly in the test and check that it contains the expected lines.

The file is configured for instance here: https://github.com/smarr/ReBench/blob/master/rebench/tests/persistency_test.py#L67-L68

And the name of the file is accessible via self._tmp_file.

Copy link
Owner

@smarr smarr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

I am a little surprised that this doesn't break any tests :)
I would somehow have expected that this might be parsed as a data point.

rebench/model/measurement.py Outdated Show resolved Hide resolved
rebench/persistence.py Outdated Show resolved Hide resolved
rebench/persistence.py Outdated Show resolved Hide resolved
rebench/model/benchmark.py Outdated Show resolved Hide resolved
rebench/tests/persistency_test.py Outdated Show resolved Hide resolved
rebench/tests/persistency_test.py Outdated Show resolved Hide resolved
rebench/tests/persistency_test.py Outdated Show resolved Hide resolved
rebench/tests/persistency_test.py Outdated Show resolved Hide resolved
rebench/tests/persistency_test.py Outdated Show resolved Hide resolved
rebench/tests/persistency_test.py Outdated Show resolved Hide resolved
rebench/tests/persistency_test.py Outdated Show resolved Hide resolved
@naomiGrew naomiGrew closed this Jul 12, 2023
@naomiGrew naomiGrew deleted the Scv-header branch July 12, 2023 10:43
@naomiGrew naomiGrew reopened this Jul 12, 2023
@naomiGrew naomiGrew force-pushed the Scv-header branch 3 times, most recently from d3bf3fb to 64eec7a Compare July 12, 2023 12:51
create csv headers

Remove trailing whitespace

add test to check if shebang line and csv header are in file and are correct

add the csv header in same place as shebang line

implement get column headers

fetch column headers for measurement

get column headers for run id

fix trailing whitespace

remove trailing whitespace

fix trailing whitespace

fix trailing whitespace

fixing whitespace

fix indentation

add line to disable pylint error and fix typo

fix typo

simplify test_check_file_lines to use deadlines()

change assertEquals to assertEqual

add extraArgs to csv header

remove trailing whitespace

add check for data values for every header
Copy link
Owner

@smarr smarr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great :) Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include CSV header in data files
2 participants