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

Fix custom anchors for kraken #2170

Merged
merged 5 commits into from Nov 13, 2023
Merged

Conversation

jchorl
Copy link
Contributor

@jchorl jchorl commented Nov 10, 2023

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md has been updated
    • Is this automated now? I saw some added actions automation around changelog in .github/workflows/changelog.yml

This change broke custom anchors for the kraken module: https://github.com/ewels/MultiQC/pull/1372/files#diff-10bfc7513ddf0bd9fd13bbb456bb610f9d45592b3a2e9337bc44a6dba05c335eR29

I don't believe find_log_files should use the anchor (which is an html detail as best I understand).

I'm not uber confident on this fix, but it does revert to how it was before, and fix our issues.

Without fix:

root@58ce75507ca9:/src/MultiQC_TestData/data/modules/kraken# multiqc -f .

  /// MultiQC ๐Ÿ” | v1.18.dev0

|           multiqc | Search path : /src/MultiQC_TestData/data/modules/kraken
|         searching | โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ” 100% 16/16  
โ•ญโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ Oops! The 'kraken' MultiQC module broke... โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฎ
โ”‚ Please copy this log and report it at https://github.com/ewels/MultiQC/issues                                โ”‚
โ”‚ Please attach a file that triggers the error. The last file found was: ./v2.0.8_beta/sample1.kraken2.report  โ”‚
โ”‚                                                                                                              โ”‚
โ”‚ Traceback (most recent call last):                                                                           โ”‚
โ”‚   File "/usr/local/lib/python3.12/site-packages/multiqc/multiqc.py", line 691, in run                        โ”‚
โ”‚     output = mod()                                                                                           โ”‚
โ”‚              ^^^^^                                                                                           โ”‚
โ”‚   File "/usr/local/lib/python3.12/site-packages/multiqc/modules/kraken/kraken.py", line 51, in __init__      โ”‚
โ”‚     for f in self.find_log_files(self.anchor, filehandles=True):                                             โ”‚
โ”‚   File "/usr/local/lib/python3.12/site-packages/multiqc/modules/base_module.py", line 139, in find_log_files โ”‚
โ”‚     for f in report.files[sp_key]:                                                                           โ”‚
โ”‚              ~~~~~~~~~~~~^^^^^^^^                                                                            โ”‚
โ”‚ KeyError: 'not_kraken'                                                                                       โ”‚
โ”‚                                                                                                              โ”‚
โ•ฐโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฏ
|           multiqc | No analysis results found. Cleaning up..
|           multiqc | MultiQC complete

With fix:

root@58ce75507ca9:/src/MultiQC_TestData/data/modules/kraken# multiqc -f .

  /// MultiQC ๐Ÿ” | v1.18.dev0

|           multiqc | Search path : /src/MultiQC_TestData/data/modules/kraken
|         searching | โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ” 100% 16/16  
|            kraken | Found 12 reports
|            kraken | Kraken2 reports of different versions were found
|           multiqc | Report      : multiqc_report.html   (overwritten)
|           multiqc | Data        : multiqc_data   (overwritten)
|           multiqc | MultiQC complete

Here is a TestData report with the fix (zipped so github allows it):

multiqc_report.zip

Here's how I tested:

  1. Add a config yaml in MultiQC_TestData/data/modules/kraken/multiqc_config.yaml:

    module_order:
      - kraken:
          anchor: not_kraken
  2. Installed multiqc locally

  3. cd to the kraken testdata dir and run

cc @vladsavelyev

@vladsavelyev vladsavelyev changed the title fix custom anchors for kraken Fix custom anchors for kraken Nov 13, 2023
@vladsavelyev
Copy link
Member

Thank you @jchorl for the pull request, and sorry about the issue!

The reason it used self.anchor there instead of a hardcoded "kraken" is that this module is also reused by bracken which would override that tag in order to reuse the code.

You are correct that it shouldn't be self.anchor though. I forgot that it can be overridden in config which would break things. Just fixed it by passing another variable sp_key to the module constructor.

Thanks again!

@vladsavelyev vladsavelyev self-requested a review November 13, 2023 09:45
@vladsavelyev vladsavelyev added the bug: module Bug in a MultiQC module label Nov 13, 2023
@vladsavelyev vladsavelyev added this to the MultiQC v1.18 milestone Nov 13, 2023
@vladsavelyev vladsavelyev merged commit 42ef22a into MultiQC:master Nov 13, 2023
6 checks passed
@jchorl
Copy link
Contributor Author

jchorl commented Nov 14, 2023

Thanks!

@vladsavelyev vladsavelyev mentioned this pull request Apr 18, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug: module Bug in a MultiQC module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants