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

Centralize sequence read operations #1730

Merged
merged 8 commits into from
Jan 30, 2025

Conversation

victorlin
Copy link
Member

@victorlin victorlin commented Jan 17, 2025

Description of proposed changes

General improvements before fixing CI failures related to sequence read.

Related issue(s)

#1727

Notable discussion threads

Checklist

  • Checks pass
  • Check if you need to add a changelog message
  • Check if you need to add tests
  • Check if you need to update docs

@victorlin victorlin self-assigned this Jan 17, 2025
@victorlin victorlin marked this pull request as ready for review January 17, 2025 23:26
@victorlin victorlin force-pushed the victorlin/centralize-sequence-read branch 4 times, most recently from cf5c8be to fbb56fe Compare January 22, 2025 01:35
Copy link

codecov bot commented Jan 22, 2025

Codecov Report

Attention: Patch coverage is 81.81818% with 4 lines in your changes missing coverage. Please review.

Project coverage is 72.99%. Comparing base (d071859) to head (91eaafe).
Report is 14 commits behind head on master.

Files with missing lines Patch % Lines
augur/export_v1.py 33.33% 2 Missing ⚠️
augur/reconstruct_sequences.py 50.00% 1 Missing ⚠️
augur/sequence_traits.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1730      +/-   ##
==========================================
+ Coverage   72.94%   72.99%   +0.05%     
==========================================
  Files          79       79              
  Lines        8317     8325       +8     
  Branches     1698     1698              
==========================================
+ Hits         6067     6077      +10     
+ Misses       1962     1960       -2     
  Partials      288      288              

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

augur/export_v1.py Outdated Show resolved Hide resolved
augur/io/sequences.py Show resolved Hide resolved
@victorlin victorlin force-pushed the victorlin/centralize-sequence-read branch 2 times, most recently from e7ed6a7 to 42a5b6f Compare January 23, 2025 19:08
The file reading is handled by augur.io.open_file, which is independent
of Biopython.
These are more useful for type checking and rendering of documentation.

I switched "Yields" to "Returns" to match the label of "Return type" in
rendered docs.
The name clashes with augur.io.read_sequences which will be used in a
future commit.

This also presents the chance to add a short comment on the similarities
and differences between the two functions.
This centralizes customization of the Biopython call. The 'format=' key
is necessary for read_sequences - without it, the value is treated as a
positional argument which is used by '*paths'.
This allows for centralized customization of the Biopython call.
This centralizes customization of the Biopython call. Added 'format='
key to improve readability.
@victorlin victorlin force-pushed the victorlin/centralize-sequence-read branch from 9476797 to 91eaafe Compare January 29, 2025 21:32
@victorlin victorlin merged commit 4ec597a into master Jan 30, 2025
36 checks passed
@victorlin victorlin deleted the victorlin/centralize-sequence-read branch January 30, 2025 17:41
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.

4 participants