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

Minor refactor on attachMultiqcReport function. #7129

Merged
merged 3 commits into from
Dec 3, 2024

Conversation

robsyme
Copy link
Contributor

@robsyme robsyme commented Nov 30, 2024

The attachMultiqcReport function was renamed as the function does not attach the multiqc report, but rather simply tests if the input object contains zero, one, or more multiqc reports and returns a single report if available and, issues a warning in the zero or >1 case.

The function was also refactored to be more resilient to the type of object passed as input. The function can now not only take a collection of zero, one, or more more multiqc reports, but will also handle the case if the input object is a Path object. Handling a path might be useful just in case somebody uses this workflow and calls .first() instead of .toList() to convert the queue channel into a value channel.

PR checklist

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • If you've added a new tool - have you followed the module conventions in the contribution docs
  • If necessary, include test data in your PR.
  • Remove all TODO statements.
  • Emit the versions.yml file.
  • Follow the naming conventions.
  • Follow the parameters requirements.
  • Follow the input/output options guidelines.
  • Add a resource label
  • Use BioConda and BioContainers if possible to fulfil software requirements.
  • Ensure that the test works with either Docker / Singularity. Conda CI tests can be quite flaky:
    • For modules:
      • nf-core modules test <MODULE> --profile docker
      • nf-core modules test <MODULE> --profile singularity
      • nf-core modules test <MODULE> --profile conda
    • For subworkflows:
      • nf-core subworkflows test <SUBWORKFLOW> --profile docker
      • nf-core subworkflows test <SUBWORKFLOW> --profile singularity
      • nf-core subworkflows test <SUBWORKFLOW> --profile conda

The attachMultiqcReport function was renamed as the function does not attach the multiqc report, but rather simply tests if the input object contains zero, one, or more multiqc reports.

The function was also refactored to be more resilient to the type of object passed as input.
The function can now not only take a collection of zero, one, or more more multiqc reports, but will also handle the case if the input object is a Path object.
@robsyme robsyme force-pushed the more-resilient-multiqc-attchments branch from 2532aaa to e4c07f3 Compare November 30, 2024 20:07
@maxulysse
Copy link
Member

Feel free to merge

@mirpedrol
Copy link
Member

Some of the tests are failing complaining about the switch: https://github.com/nf-core/modules/actions/runs/12138791850/job/33845006274?pr=7129

@mirpedrol mirpedrol added this pull request to the merge queue Dec 3, 2024
Merged via the queue into master with commit 8540068 Dec 3, 2024
29 checks passed
@mirpedrol mirpedrol deleted the more-resilient-multiqc-attchments branch December 3, 2024 13:17
LouisLeNezet pushed a commit to LouisLeNezet/modules that referenced this pull request Dec 4, 2024
* Minor refactor on attachMultiqcReport function.

The attachMultiqcReport function was renamed as the function does not attach the multiqc report, but rather simply tests if the input object contains zero, one, or more multiqc reports.

The function was also refactored to be more resilient to the type of object passed as input.
The function can now not only take a collection of zero, one, or more more multiqc reports, but will also handle the case if the input object is a Path object.

* fix switch brackets

---------

Co-authored-by: Júlia Mir Pedrol <mirp.julia@gmail.com>
@SPPearce
Copy link
Contributor

SPPearce commented Dec 5, 2024

The VSCode plugin is complaining about the switch:
image
Complaining about:
Unexpected input: '\n'

@robsyme
Copy link
Contributor Author

robsyme commented Dec 6, 2024

Great point Simon. Fixed with #7168

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.

4 participants