split summary.csv into summary and summary_extended #890
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
artoonie
force-pushed
the
feature/issue-856_summary-extended
branch
from
f13accc to
9ba3387
Compare
artoonie
force-pushed
the
feature/issue-856_summary-extended
branch
from
9ba3387 to
e61e9df
Compare
artoonie
changed the title
artoonie
changed the title
artoonie
changed the title
yezr
changed the title
yezr
reviewed
Sep 30, 2024
Testing
|
artoonie
commented
Oct 3, 2024
artoonie
force-pushed
the
feature/issue-856_summary-extended
branch
from
6a44de8 to
3b4eaca
Compare
artoonie
force-pushed
the
feature/issue-856_summary-extended
branch
from
b16d565 to
8b8fe60
Compare
…ry (#894) * refactor how filenames are chosen * move files to new recommended name * rename ResultFile to ResultTypeAndSlice, and CVR_CDF to CDF_CVR * address additional PR comments * rename files: CVR_CDF to CDF_CVR * refator to make explicit the difference between a results file (what we previously referred to as the summary files) and an output file (the results files plus rctab_cvr and cdf_cvr) * fix comment * refactor how filenames are chosen * move files to new recommended name * rename ResultFile to ResultTypeAndSlice, and CVR_CDF to CDF_CVR * address additional PR comments * rename files: CVR_CDF to CDF_CVR * refator to make explicit the difference between a results file (what we previously referred to as the summary files) and an output file (the results files plus rctab_cvr and cdf_cvr) * fix comment * PR comments addressed and tests fixed * add warning message for sanitized slice filename collision * fix logging message location --------- Co-authored-by: yezr <[email protected]>
yezr
approved these changes
Nov 5, 2024
There was a problem hiding this comment.
big ups to @alyssahursh, @artoonie and @nurse-the-code for all the work on this. LGTM!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #856
Adds tests for every summary.csv file without bloating the repo by including that mostly-duplicate data.