-
Notifications
You must be signed in to change notification settings - Fork 583
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
MultiQC 1.17 fails to recognize fastp files #2138
Comments
Given below is the verbose log with multiqc 1.16
|
Thank you @tamuanand for reporting the bug. It indeed was introduced while attempting to fix another issue. Here is the fix: #2139 It now correctly parses the "command" field in the JSON when it uses I also added your test example into the test data repo: MultiQC/test-data#295 |
I am not sure I am happy with this fix.. With previous versions of multiqc, with this command below, I would get the General Stats table to show up as HG001 for fastp related stats.
With the current fix, it shows up in General Stats table as HG001_1.
Is there a way to have it reported as before - please. Thanks in advance. |
I'm not sure though there is a good way to find that the sample name is The problem here is that the default file name for the JSON file that we need is just I can't think of an obvious generalizable solution here, without involving configuration options (e.g. Can you think of a generalizable solution? |
How about this
|
That should work. Though I'm a bit hesitant though because normally a module checks one place for the sample name, which makes the behaviour transparent. And here, the behaviour would be different when the file name is exactly |
I agree - but my question is "why fix/change something that has been working for years now" - especially with something like fastp I have this in my multiqc_config.yaml - but it does not work
Does fn_clean_trim take regex? Something like
|
I know what you feel, that's annoying that something suddenly stops working for no good reason. But it wasn't always working: #2031 And the way it was, it worked often by accident, e.g. it wouldn't have behaved correctly with inputs with Anyway, we should have better tested all possible scenarios. Hopefully it's a good opportunity now, thanks to you for bringing back the discussion. Regarding the config options, you can just use the following in extra_fn_clean_trim: "_1" It will remove Note that if you want to use |
Just checking - will there be some more fixes based on what we have discussed above. Thanks. |
@tamuanand, I pushed another change based on our discussion, as you proposed, see the pull-request: #2139 |
I can confirm that this latest patch/fix works as before without me having to do any config changes like this
Will there be a 1.17.1 release or something like that? Thanks |
This kind of discussion is tricky as everyone uses bioinformatics tools differently - other users might call the fastp results My preference is to use the FastQ in the command line as the top priority - this is the same behaviour as we have in most other modules, such as Trimmomatic, Cutadapt, FastQC etc. I think that consistency with other modules should be maintained where possible. Trimmomatic has a module-specific configuration option |
Is there a solution for that problem by now? For now, all I can do is manually modify the jason file from:
to:
|
@yeroslaviz, the solution is merged into the master branch of the repository, but will be only available in the installable package after the next release ( |
Description of bug
Hi @vladsavelyev and @ewels
MultQC 1.17 fails to detect fastp files - the same works with MultiQC 1.16
I will attach the verbose output from MultiQC 1.16 as a comment so that you can see the difference(s).
File that triggers the error
HG001.fastp.json
HG002_son.fastp.json
HG003_father.fastp.json
HG004_mother.fastp.json
MultiQC Error log
Before submitting
The text was updated successfully, but these errors were encountered: