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 files for CollectAlignmentSummaryMetrics and FastqToSam #1859

Merged
merged 5 commits into from Feb 15, 2023

Conversation

kachulis
Copy link
Contributor

Currently, if input files to the tools have 0 reads, various exceptions are thrown by CollectAlignmentSummaryMetrics and FastqToSam. This PR adds an option to allow FastqToSam to produce an empty Sam when passed an empty fastq instead of throwing an exception, and does not try to plot readlength distribution in CollectAlignmentSummaryMetrics when there are 0 reads (which currently hits an r error).

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.

I recommend writing a simple test for the expected behavior with empty files.

@kachulis
Copy link
Contributor Author

@tmelman I added a few tests. Turns out there was a bug in this seemingly trivial change, so that was a good call!

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.

Glad the tests revealed the bug, tests and code look good to me, ok to merge.

My one last comment would be make sure you're logging warnings when you mean warnings, and not when you want it to throw an error. But if this is meant to tolerate empty histogram outputs, then this is fine by me.

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.

None yet

2 participants