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

[vcf] replace augur's read_vcf with TreeTime's #1366

Merged
merged 1 commit into from
Jan 25, 2024

Conversation

jameshadfield
Copy link
Member

@jameshadfield jameshadfield commented Dec 20, 2023

The strain names are available from TreeTime's function of the same name. This function is preferable to (a) reduce duplicated logic and (b) because it performs more thorough validation of the VCF file.

Aside: Augur's read_vcf function (which has moved around quite a lot) was first introduced by 278a30e in May
2018. This returned a tuple of (strain_names, strain_names.copy()) and this return signature has been kept during refactors despite us having since stopped using the second argument, except for in tests!

Closes #1357

  • Checks pass.
  • If making user-facing changes, add a message in CHANGES.md summarizing the changes in this PR

@jameshadfield jameshadfield marked this pull request as ready for review December 20, 2023 02:44
@jameshadfield jameshadfield requested a review from a team December 20, 2023 02:44
Copy link

codecov bot commented Dec 20, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d83d671) 66.68% compared to head (70ee412) 66.65%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1366      +/-   ##
==========================================
- Coverage   66.68%   66.65%   -0.03%     
==========================================
  Files          69       69              
  Lines        7330     7321       -9     
  Branches     1801     1799       -2     
==========================================
- Hits         4888     4880       -8     
  Misses       2172     2172              
+ Partials      270      269       -1     

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

The strain names are available from TreeTime's function of the same
name. This function is preferable to (a) reduce duplicated logic and
(b) because it performs more thorough validation of the VCF file.

Aside: Augur's `read_vcf` function (which has moved around quite a lot)
was first introduced by 278a30e in May
2018. This returned a tuple of `(strain_names, strain_names.copy())` and
this return signature has been kept during refactors despite us having
since stopped using the second argument, except for in tests!

Closes #1357
@jameshadfield jameshadfield merged commit 2a08326 into master Jan 25, 2024
23 checks passed
@jameshadfield jameshadfield deleted the james/replace-augur-read-vcf branch January 25, 2024 00:32
@victorlin victorlin mentioned this pull request Jan 30, 2024
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.

[vcf] replace VCF reading with TreeTime's read_vcf
1 participant