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

Allow selection of metadata ID column #1240

Merged
merged 3 commits into from
Jun 14, 2023

Conversation

victorlin
Copy link
Member

@victorlin victorlin commented Jun 14, 2023

Description of proposed changes

See commit messages.

Related issue(s)

Resolves #1237.

Testing

No tests added.

Checklist

  • Add a message in CHANGES.md summarizing the changes in this PR that are end user focused. Keep headers and formatting consistent with the rest of the file.

Since it is not a positional argument.
Previously, this flag to customize the default value was only available
in augur filter. Add it to other subcommands to parallel the existing
support for --metadata-valid-delimiters which serves a similar purpose.
@victorlin victorlin requested a review from a team June 14, 2023 20:36
Copy link
Member Author

Choose a reason for hiding this comment

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

Re: @jameshadfield in original issue description:

We don't need to add it to export v1.

I opted to add this to export v1 to keep things consistent, plus it's an easy change.

Copy link
Member Author

Choose a reason for hiding this comment

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

Update: Reverting this in #1265 (specifically 0354812).

@codecov
Copy link

codecov bot commented Jun 14, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.02 🎉

Comparison is base (7139595) 68.88% compared to head (0b64f5e) 68.90%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1240      +/-   ##
==========================================
+ Coverage   68.88%   68.90%   +0.02%     
==========================================
  Files          64       64              
  Lines        6939     6944       +5     
  Branches     1693     1693              
==========================================
+ Hits         4780     4785       +5     
  Misses       1854     1854              
  Partials      305      305              
Impacted Files Coverage Δ
augur/filter/_run.py 94.47% <ø> (ø)
augur/export_v1.py 74.27% <100.00%> (+0.10%) ⬆️
augur/export_v2.py 71.95% <100.00%> (+0.04%) ⬆️
augur/frequencies.py 52.38% <100.00%> (+0.38%) ⬆️
augur/refine.py 65.85% <100.00%> (+0.16%) ⬆️
augur/traits.py 84.37% <100.00%> (+0.12%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@joverlee521 joverlee521 left a comment

Choose a reason for hiding this comment

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

Thank you for working on this! LGTM by inspection.

@victorlin victorlin merged commit cfb5255 into master Jun 14, 2023
@victorlin victorlin deleted the victorlin/update-read-metadata-subcommands branch June 14, 2023 22:44
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.

Allow selection of metadata ID column
2 participants