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

Allow empty input in TransferReadTags #8198

Merged
merged 2 commits into from
Feb 15, 2023
Merged

Conversation

kachulis
Copy link
Contributor

TransferReadTags will throw an exception if the unaligned bam has no read. This pr will allow the unaligned bam to have 0 reads, as long as the aligned bam also has 0 reads, which will result in an output bam which is just a copy of the aligned bam, with 0 reads.

@codecov
Copy link

codecov bot commented Feb 10, 2023

Codecov Report

Merging #8198 (7608bd2) into master (d1a4f94) will decrease coverage by 0.007%.
The diff coverage is 0.000%.

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

Additional details and impacted files
@@               Coverage Diff               @@
##              master     #8198       +/-   ##
===============================================
- Coverage     86.658%   86.651%   -0.007%     
- Complexity     39024     39155      +131     
===============================================
  Files           2337      2346        +9     
  Lines         182961    183623      +662     
  Branches       20079     20148       +69     
===============================================
+ Hits          158550    159111      +561     
- Misses         17365     17449       +84     
- Partials        7046      7063       +17     
Impacted Files Coverage Δ
.../hellbender/tools/walkers/qc/TransferReadTags.java 72.549% <0.000%> (-2.961%) ⬇️
...lbender/tools/sv/cluster/CanonicalSVCollapser.java 80.635% <0.000%> (-6.836%) ⬇️
...roadinstitute/hellbender/tools/sv/SVTestUtils.java 90.274% <0.000%> (-6.426%) ⬇️
...ellbender/tools/sv/cluster/CanonicalSVLinkage.java 83.146% <0.000%> (-6.043%) ⬇️
...stitute/hellbender/tools/sv/SVCallRecordUtils.java 79.167% <0.000%> (-4.518%) ⬇️
...ender/utils/variant/GATKSVVariantContextUtils.java 93.939% <0.000%> (-2.835%) ⬇️
...der/tools/walkers/sv/SVClusterIntegrationTest.java 98.974% <0.000%> (-0.522%) ⬇️
.../hellbender/tools/walkers/sv/SVAnnotateEngine.java 89.434% <0.000%> (-0.347%) ⬇️
...stitute/hellbender/tools/walkers/sv/SVCluster.java 89.773% <0.000%> (ø)
...titute/hellbender/utils/GenotypeUtilsUnitTest.java 94.624% <0.000%> (ø)
... and 24 more

Copy link
Contributor

@tmelman tmelman left a comment

Choose a reason for hiding this comment

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

Add test where 1 file has 0 and 1 file has x reads to see that it fails with UserException; add test where both have 0 reads to see that warning is raised and output has 0 reads.

@kachulis
Copy link
Contributor Author

@tmelman I have added tests, so this is ready for another round of review.

Copy link
Contributor

@tmelman tmelman left a comment

Choose a reason for hiding this comment

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

LGTM

@kachulis kachulis merged commit a88ac4e into master Feb 15, 2023
@kachulis kachulis deleted the ck_transferreadtags_empty_input branch February 15, 2023 17:35
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