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

3.3.1 release #764

Open
wants to merge 46 commits into
base: master
Choose a base branch
from
Open

3.3.1 release #764

wants to merge 46 commits into from

Conversation

jfy133
Copy link
Member

@jfy133 jfy133 commented Feb 10, 2025

Just bug fixes:

Added

Changed

Fixed

  • #748 - Fix broken phix reference channel when skipping phix removal (reported by @amizeranschi, fix by @muabnezor)
  • #752 - Fix QUAST results not being displayed when skipping certain steps (reported by @amizeranschi, fix by @jfy133)
  • #753 - Fix iGenomes reference support for host removal reference genome (reported by @Thomieh73, fix by @jfy133)
  • #759 - Fixed parameters that allow both files or directories to not error with directories, and general file input validation improvements (repoted by @mjfi2sb3, fix by @jfy133)

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 pipeline conventions in the contribution docs
  • If necessary, also make a PR on the nf-core/mag branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core pipelines lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Check for unexpected warnings in debug mode (nextflow run . -profile debug,test,docker --outdir <OUTDIR>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

jfy133 and others added 30 commits December 19, 2024 14:13
Important! Template update for nf-core/tools v3.1.1
Use channel empty when no phix reference supplied because skipping
Important! Template update for nf-core/tools v3.1.2
CHANGELOG.md Outdated Show resolved Hide resolved
@jfy133 jfy133 marked this pull request as ready for review February 10, 2025 08:36
Copy link

github-actions bot commented Feb 10, 2025

nf-core pipelines lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 47542c7

+| ✅ 329 tests passed       |+
#| ❔   2 tests were ignored |#
!| ❗  52 tests had warnings |!

❗ Test warnings:

❔ Tests ignored:

✅ Tests passed:

Run details

  • nf-core/tools version 3.2.0
  • Run at 2025-02-10 08:37:24

Copy link
Contributor

@heuermh heuermh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nice work!

Copy link
Member

@erikrikarddaniel erikrikarddaniel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks all good to me!
A comment and a question:

  • I could have commented earlier but as I see it now: The subway map gives me the impression that it's XOR between e.g. CheckM, Prokka and GTDB-Tk.
  • I note that you added a comma after the last parameter to process calls -- are we supposed to do that now?

@jfy133
Copy link
Member Author

jfy133 commented Feb 11, 2025

Looks all good to me! A comment and a question:

* I could have commented earlier but as I see it now: The subway map gives me the impression that it's XOR between e.g. CheckM, Prokka and GTDB-Tk.

* I note that you added a comma after the last parameter to process calls -- are we supposed to do that now?

Thanks @erikrikarddaniel !

  1. Could you explain why it gives you that imporession? I don't really see what you mean.
  2. That's from the language server auto-formatter 🤷

@erikrikarddaniel
Copy link
Member

Looks all good to me! A comment and a question:

* I could have commented earlier but as I see it now: The subway map gives me the impression that it's XOR between e.g. CheckM, Prokka and GTDB-Tk.

* I note that you added a comma after the last parameter to process calls -- are we supposed to do that now?

Thanks @erikrikarddaniel !

1. Could you explain why it gives you that imporession? I don't really see what you mean.

Because all those processes are shown as parallel paths. In other parts of the diagram there are places when one can skip a step indicated by a bubble to the step that can be skipped. It hence looks to me like one takes one path through those alternatives. But I'm certainly not an authority on this.

2. That's from the language server auto-formatter 🤷

I see, good to know.

@jfy133
Copy link
Member Author

jfy133 commented Feb 11, 2025

Looks all good to me! A comment and a question:

* I could have commented earlier but as I see it now: The subway map gives me the impression that it's XOR between e.g. CheckM, Prokka and GTDB-Tk.

* I note that you added a comma after the last parameter to process calls -- are we supposed to do that now?

Thanks @erikrikarddaniel !

1. Could you explain why it gives you that imporession? I don't really see what you mean.

Because all those processes are shown as parallel paths. In other parts of the diagram there are places when one can skip a step indicated by a bubble to the step that can be skipped. It hence looks to me like one takes one path through those alternatives. But I'm certainly not an authority on this.

2. That's from the language server auto-formatter 🤷

I see, good to know.

  1. Hrmm I guess I can see what you mean now.... would making the 'bubble' lines dashed help?

@erikrikarddaniel
Copy link
Member

[...]

1. Hrmm I guess I can see what you mean now.... would making the 'bubble' lines dashed help?

Not really, but I don't want you to waste too much time on this... My view of these diagrams is the "particle view" of quantum physics, not the "wave view", i.e. I can take one path through the diagram. I suppose you made it this way because these steps are executed in parallel though. But, no big deal!

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