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

[augur ancestral] arg parsing improvements #1344

Merged
merged 4 commits into from Dec 10, 2023

Conversation

jameshadfield
Copy link
Member

@jameshadfield jameshadfield commented Nov 29, 2023

Three Four small commits - see messages in each for details.

This PR is potentially a backwards incompatible change, and the changelog has been updated accordingly.

Closes #1343

@jameshadfield jameshadfield requested a review from a team November 29, 2023 02:30
Copy link

codecov bot commented Nov 29, 2023

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (7cb3848) 65.80% compared to head (5b0545f) 65.94%.

Files Patch % Lines
augur/ancestral.py 60.71% 9 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1344      +/-   ##
==========================================
+ Coverage   65.80%   65.94%   +0.14%     
==========================================
  Files          68       68              
  Lines        7153     7156       +3     
  Branches     1756     1758       +2     
==========================================
+ Hits         4707     4719      +12     
+ Misses       2182     2173       -9     
  Partials      264      264              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jameshadfield jameshadfield force-pushed the james/ancestral-arg-improvements branch 2 times, most recently from 9566edd to a529731 Compare December 4, 2023 21:00
augur/ancestral.py Outdated Show resolved Hide resolved
tests/functional/ancestral/cram/invalid-args.t Outdated Show resolved Hide resolved
augur/ancestral.py Outdated Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
Check that provided arguments are compatible. Where possible we use
argparse built-ins, but they don't cover everything we want to check.
This checking isn't intended to replace conditional checks in the code
which check for arg existence (as demonstrated by the additional
conditional added here), however by checking for invalid combinations
up-front we can exit quickly.
This is consistent with augur's general behaviour of "request a file for
it to be produced". Moreover, the previous code would ~always result in
the VCF input file being overwritten.

This is potentially a backwards compatible change. The number of users
expected to be affected is minimal (perhaps zero).

Closes #1343.
This is the preferred method rather than printing to STDERR and exiting
For VCF alignments, `--vcf-reference` becomes the 'ref' variable and
for FASTA alignments, `--root-sequence` becomes the 'ref' variable.
If both was provided then (previously) one was silently ignored, but
they are now mutually exclusive.

The argument help messages are also improved.
@jameshadfield jameshadfield merged commit 657cd7b into master Dec 10, 2023
24 checks passed
@jameshadfield jameshadfield deleted the james/ancestral-arg-improvements branch December 10, 2023 20:33
jameshadfield added a commit that referenced this pull request Dec 11, 2023
The move to "Creating files in the initial working directory" is
motivated by
<#1344 (comment)>
and <#1176>.

Additionally, I remove the pushd commands which were confusing (there
were multiple!) and use variables to refer to common directories to
improve readability.
jameshadfield added a commit that referenced this pull request Dec 19, 2023
The move to "Creating files in the initial working directory" is
motivated by
<#1344 (comment)>
and <#1176>.

Additionally, I remove the pushd commands which were confusing (there
were multiple!) and use variables to refer to common directories to
improve readability.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[augur ancestral] input VCF alignment is often overwritten
2 participants