-
-
Notifications
You must be signed in to change notification settings - Fork 70
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 an option to store sample base qualities in the QT for FastqToBam #933
Conversation
@@ -322,6 +322,24 @@ class FastqToBamTest extends UnitSpec { | |||
recs(3).basesString shouldBe "CCCCCCCCCC" | |||
} | |||
|
|||
it should "extract sample barcode qualities when requested" in { |
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.
@galaxy001, take a look at the inputs and outputs and see if this suffices.
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the GitHub App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## main #933 +/- ##
=======================================
Coverage 95.60% 95.60%
=======================================
Files 126 126
Lines 7305 7307 +2
Branches 517 507 -10
=======================================
+ Hits 6984 6986 +2
Misses 321 321
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
@@ -92,6 +92,7 @@ class FastqToBam | |||
@arg(flag='s', doc="If true, queryname sort the BAM file, otherwise preserve input order.") val sort: Boolean = false, | |||
@arg(flag='u', doc="Tag in which to store molecular barcodes/UMIs.") val umiTag: String = ConsensusTags.UmiBases, | |||
@arg(flag='q', doc="Tag in which to store molecular barcode/UMI qualities.") val umiQualTag: Option[String] = None, | |||
@arg(doc="Store the sample barcode qualities in the QT Tag.") val storeSampleBarcodeQualities: Boolean = false, |
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.
Why there is no flag
? What will the option char for enabling QT
?
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.
Use the full command line option --store-sample-barcode-qualities
. I don't think there's a flag that makes sense, and likely since folks are implementing this in a workflow software, I don't think it's too onerous. If you really want one, I'd pick x
since it's sufficiently different than the others.
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.
I'd probably just add -Q
if it were up to me
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.
Requested one minor test addition.
@@ -92,6 +92,7 @@ class FastqToBam | |||
@arg(flag='s', doc="If true, queryname sort the BAM file, otherwise preserve input order.") val sort: Boolean = false, | |||
@arg(flag='u', doc="Tag in which to store molecular barcodes/UMIs.") val umiTag: String = ConsensusTags.UmiBases, | |||
@arg(flag='q', doc="Tag in which to store molecular barcode/UMI qualities.") val umiQualTag: Option[String] = None, | |||
@arg(doc="Store the sample barcode qualities in the QT Tag.") val storeSampleBarcodeQualities: Boolean = false, |
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.
I'd probably just add -Q
if it were up to me
recs(2).apply[String]("BC") shouldBe "GAA-TCG" | ||
recs(2).apply[String]("QT") shouldBe "CDE IJK" | ||
} | ||
|
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.
Could you either add a second pass here, or assert in one of the other tests, that if you don't supply a value for storeSampleBarcodeQualities
that the QT
tag doesn't get set with anything?
Alternative to #932 to implement #931