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

GH1563: add defaults for some columns #1595

Merged
merged 5 commits into from
Jan 15, 2022
Merged

Conversation

yanick
Copy link
Contributor

@yanick yanick commented Nov 23, 2021

This is for #1563

Big ⚠️ -> I am not using this package directly, so I'm likely to have made assumptions that are wrong.

This being said, the case reported in the issue is going 💥 because two columns are not present in a csv file. I'm assuming that is a valid schema of that file, and that it's reasonable to set the values of those columns to be '0' if not found.

If I'm mistaken and the csv absolutely needs to have those columns, the patch will have to be changed to politely tell the user that their format is wonky. :-)

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md has been updated

@skanwal
Copy link

skanwal commented Dec 5, 2021

@yanick a quick comment as you mentioned you aren't using this module directly - if you've time, I think this comment can also be tackled as part of your PR to read correct read stats column from a different file Quality_Stats.csv, which is very useful metrics.

@ewels
Copy link
Member

ewels commented Jan 15, 2022

Many thanks for this @yanick!

I continued your PR on a bit, adding in parsing functionality for the new Quality_Metrics.csv file mentioned by @Garrett-Ng in #1563 (comment) (as suggested by @skanwal above). The module should now work with both types of output and give comparable reports, based on testing with the example data provided in #1563.

Please all give this new code a try and let me know if you hit any problems 👍🏻

Phil

@ewels ewels enabled auto-merge January 15, 2022 20:05
@ewels ewels linked an issue Jan 15, 2022 that may be closed by this pull request
@ewels ewels merged commit 727cba2 into MultiQC:master Jan 15, 2022
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.

BCL Convert moduleKeyError: # of >= Q30 Bases (PF)
3 participants