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

[ORCA-175] Add CsvUpdater and update_csv command #39

Merged
merged 19 commits into from
Jun 14, 2023

Conversation

BWMac
Copy link
Contributor

@BWMac BWMac commented Jun 9, 2023

This PR follows on from #38 and implements a CsvUpdater class (and needed tests) which will be used to update the input CSV file for the DCQC run with a new column indicating the SuiteStatus for each file following processing. It also adds a new CLI command update_csv which will apply this new class to make the needed changes. The logic for updating CSV files is agnostic to single/multi-target QC runs, and uses the file URL as an ID to evaluate all suites generated against each individual target file. Necessary example files input.csv, output.csv, and suites.json are also added to tests/data.

suite_abc.py is also updated to include a get_status method for accessing the SuiteStatus from outside of the class.

edit: I am marking this as ready for review after successfully testing these changes in nf-dcqc.

@swarmia
Copy link

swarmia bot commented Jun 9, 2023

@BWMac BWMac changed the title adds SuiteStatus and applies changes as needed [ORCA-175] Add CsvUpdater and update_csv command Jun 9, 2023
@BWMac BWMac mentioned this pull request Jun 9, 2023
@codecov
Copy link

codecov bot commented Jun 9, 2023

Codecov Report

Merging #39 (c21bc9b) into main (a46b491) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main       #39   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           22        23    +1     
  Lines         1090      1143   +53     
  Branches       126       189   +63     
=========================================
+ Hits          1090      1143   +53     
Impacted Files Coverage Δ
src/dcqc/main.py 100.00% <100.00%> (ø)
src/dcqc/suites/suite_abc.py 100.00% <100.00%> (ø)
src/dcqc/updaters.py 100.00% <100.00%> (ø)

@thomasyu888 thomasyu888 changed the base branch from main to bwmac/orca-175/update_input_with_status June 12, 2023 20:55
@BWMac BWMac requested a review from thomasyu888 June 12, 2023 23:21
Copy link
Contributor

@thomasyu888 thomasyu888 left a comment

Choose a reason for hiding this comment

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

Nice! This is my initial feedback - will review more tomorrow.

src/dcqc/suites/suite_abc.py Outdated Show resolved Hide resolved
src/dcqc/main.py Show resolved Hide resolved
src/dcqc/updaters.py Show resolved Hide resolved
src/dcqc/updaters.py Outdated Show resolved Hide resolved
src/dcqc/updaters.py Outdated Show resolved Hide resolved
src/dcqc/updaters.py Show resolved Hide resolved
src/dcqc/updaters.py Outdated Show resolved Hide resolved
src/dcqc/updaters.py Outdated Show resolved Hide resolved
Base automatically changed from bwmac/orca-175/update_input_with_status to main June 13, 2023 16:02
@BWMac BWMac marked this pull request as ready for review June 13, 2023 19:59
tests/conftest.py Outdated Show resolved Hide resolved
Copy link
Contributor

@thomasyu888 thomasyu888 left a comment

Choose a reason for hiding this comment

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

🔥 @BWMac Nice work! I'm going to pre-approve, but please add the missing test case for when there is no status.

@BWMac BWMac merged commit b9d016f into main Jun 14, 2023
@BWMac BWMac deleted the bwmac/orca-175/add_csv_updater branch June 14, 2023 00:04
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.

2 participants