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

feat: add SupplementaryAlignment constructor from read #85

Merged
merged 3 commits into from
Feb 9, 2024

Conversation

msto
Copy link
Contributor

@msto msto commented Jan 30, 2024

I thought it'd be helpful to abstract away the logic for validating the presence of an SA tag and type-checking, since that's going to be necessary most of the time parse_sa_tag is used in practice

Copy link

codecov bot commented Jan 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b24ea84) 93.29% compared to head (db14344) 93.33%.
Report is 2 commits behind head on main.

❗ Current head db14344 differs from pull request most recent head f172954. Consider uploading reports for the commit f172954 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #85      +/-   ##
==========================================
+ Coverage   93.29%   93.33%   +0.03%     
==========================================
  Files          32       32              
  Lines        3343     3363      +20     
  Branches      618      619       +1     
==========================================
+ Hits         3119     3139      +20     
  Misses        149      149              
  Partials       75       75              

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

Copy link
Member

@nh13 nh13 left a comment

Choose a reason for hiding this comment

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

LGTM, thank-you!

@msto msto force-pushed the ms_supp-aln-constructor branch from db14344 to f172954 Compare January 31, 2024 14:03
Comment on lines +581 to +590
"""
Construct a list of SupplementaryAlignments from the SA tag in a pysam.AlignedSegment.

Args:
read: An alignment. The presence of the "SA" tag is not required.

Returns:
A list of all SupplementaryAlignments present in the SA tag.
If the SA tag is not present, or it is empty, an empty list will be returned.
"""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nh13 to follow up on our slack convo, I added some detail to the docstring to clarify the behavior when the SA tag is absent.

lmk if you have any suggested edits to the docs before I merge

@msto msto requested a review from nh13 January 31, 2024 14:05
@msto msto merged commit 94b3696 into main Feb 9, 2024
5 checks passed
@msto msto deleted the ms_supp-aln-constructor branch February 9, 2024 14:38
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