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.

Sign up for GitHub

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

Jump to bottom

Revert "Update FastQC and UMItools modules" #1148

Merged
merged 1 commit into from
Jan 3, 2024
Merged

Revert "Update FastQC and UMItools modules" #1148

merged 1 commit into from
Jan 3, 2024

Conversation

pinin4fjords
Copy link
Member

Reverts #1138 at @drpatelh 's request.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 3, 2024 •
edited
Loading

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit c781080

+| ✅ 146 tests passed       |+
#| ❔   6 tests were ignored |#
!| ❗   4 tests had warnings |!

❗ Test warnings:

  • files_exist - File not found: .github/workflows/awstest.yml
  • files_exist - File not found: .github/workflows/awsfulltest.yml
  • pipeline_todos - TODO string in methods_description_template.yml: #Update the HTML below to your preferred methods description, e.g. add publication citation for this pipeline
  • pipeline_todos - TODO string in WorkflowRnaseq.groovy: Optionally add in-text citation tools to this list.

❔ Tests ignored:

  • files_unchanged - File ignored due to lint config: assets/email_template.html
  • files_unchanged - File ignored due to lint config: assets/email_template.txt
  • files_unchanged - File ignored due to lint config: lib/NfcoreTemplate.groovy
  • files_unchanged - File ignored due to lint config: .gitignore or .prettierignore or pyproject.toml
  • actions_awstest - 'awstest.yml' workflow not found: /home/runner/work/rnaseq/rnaseq/.github/workflows/awstest.yml
  • multiqc_config - multiqc_config

✅ Tests passed:

Run details

  • nf-core/tools version 2.11.1
  • Run at 2024-01-03 14:20:21

@drpatelh
Copy link
Member

drpatelh commented Jan 3, 2024

Sorry (and Happy New Year) @mahesh-panchal @maxulysse! Going to revert #1148 and try and push those changes to the https://github.com/nf-core/rnaseq/tree/nf-test branch instead. The plan is to create a clean, nf-test release in the near future that we can use to show the community how to adopt this in their own pipelines.

For now, we need to keep the dev branch for small fixes / patches and a few have propped up that we need to get out sooner rather than later.

We can cherry pick the module updates that you pushed and use nf-core modules patch for now to ensure the core problem is fixed.

@MatthiasZepper
Copy link
Member

MatthiasZepper commented Jan 3, 2024 •
edited
Loading

Reverts #1138 at @drpatelh 's request.

Can you please elaborate what the rationale is to revert?

With Nextflow 23.10, a ton of modules, which silently misuse the home directory as writeable temporary cache (most common offenders are Python libraries such as Matplotlib) crash, if not run with NXF_SINGULARITY_HOME_MOUNT=true, because the --no-home setting became default. This for example affects umitools.

Therefore, I was very happy that Mahesh addressed this issue quickly.

Edit: Sorted! Already had started typing before @drpatelh added his explanation.

@drpatelh
Copy link
Member

drpatelh commented Jan 3, 2024

The plan is to create a clean, nf-test release in the near future that we can use to show the community how to adopt this in their own pipelines.

This is the main rationale. To be able to point users / developers now and in the future to a clean release to show how nf-test can be adopted in Nextflow pipelines. This is partly why we have been systematically updating all of the modules used in this pipeline on nf-core/modules as part of the last 2 Hackathons. Merging #1138 means that we now have some modules that have nf-test components and others that don't. Given we need to create a patch release soon to fix the issue you mentioned as well as others (milestone) this also means the next release will have some nf-test components but not all of them.

I agree we need to fix the issue you mentioned but we can cherry pix the version command changes and patch the modules instead which will serve both purposes.

@drpatelh
Copy link
Member

drpatelh commented Jan 3, 2024

Merging this in and will fix the tests on dev in a follow-up PR.

@drpatelh drpatelh merged commit c261e9c into dev Jan 3, 2024
19 of 29 checks passed
@drpatelh drpatelh mentioned this pull request Jan 3, 2024
Merged
@drpatelh
Copy link
Member

drpatelh commented Jan 3, 2024

Fixed tests in #1149

@drpatelh drpatelh deleted the revert-1138-dev branch January 21, 2024 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pinin4fjords @drpatelh @MatthiasZepper @adamrtalbot