-
Notifications
You must be signed in to change notification settings - Fork 22
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
fix: infer library type relationship #157
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #157 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 13 13
Lines 1109 1131 +22
=========================================
+ Hits 1109 1131 +22 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me (minus the very few minor comments), but I have to be honest in that it is a bit hard to follow the code in the library type model since the introduction of the mappings. I am pretty confident given the extensive tests, but I propose that you play this through with some small real world examples as well to check if this really behaves as desired from end to end.
To summarize: refactored When iterating through the reads, the problem during For checking the ratio between the concordant and the aligned reads (for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving, but please take care of the two comments first 🙏
Oh, and please update the PR description to reflect all of the latest changes. As far as I understood, you also fixed some things that never really worked. |
Description
first_mate
andsecond_mate
when it can correctly be inferred from seq_id's (when seq_ids are not empty)library source
is either inferred or given by--tax-id
argument (in case oflibrary type
as well asread orientation
inference)--outSAMorder PairedKeepInputOrder
tomapping.py
to keep input order of reads when running STAR with multiple threadsIn the case of paired-end libraries, the logic should be:
seq_ids
of first and second pair, to decide if they fit the Casava format, and the lib type and relationship can be determinedlibrary_source
is knownnot_mates
ornot_available
, infer the orientation from the separately aligned resultssplit_mates
, align the samples in paired-end mode, but only iflibrary_source
is knownFixes #153
Type of change
Checklist
Please carefully read these items and tick them off if the statements are true
or do not apply.
warnings
have added type annotations for any local variables that are non-trivial,
potentially ambiguous or might otherwise benefit from explicit typing.
methods/functions or updated previously existing ones
works
reduced the code coverage relative to the previous state
by the proposed changes
If for some reason you are unable to tick off all boxes, please leave a
comment explaining the issue you are facing so that we can work on it
together.