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

pypgx/runngspipeline #6823

Merged
merged 11 commits into from
Jan 13, 2025
Merged

pypgx/runngspipeline #6823

merged 11 commits into from
Jan 13, 2025
+375 −0

Conversation

Jorisvansteenbrugge
Copy link
Contributor

PR checklist

Closes #5987
This PR adds a new module: pypgx/runngspipeline

  • 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
  • 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

@Jorisvansteenbrugge Jorisvansteenbrugge added new module Adding a new module Ready for Review help wanted Extra attention is needed and removed Ready for Review labels Oct 22, 2024
@Jorisvansteenbrugge
Copy link
Contributor Author

For some reason the conda based tests are not working..

@famosab
Copy link
Contributor

famosab commented Oct 31, 2024 •
edited
Loading

Can you run the test locally with the conda profile?
nf-core modules test pypgx/runngspipeline --profile conda -u

@Jorisvansteenbrugge
Copy link
Contributor Author

Can you run the test locally with the conda profile? nf-core modules test pypgx/runngspipeline --profile conda -u

@famosab Thank you for taking a look! I get the same error unfortunately.. I wonder if this is the same issue as #4234 opened by @edmundmiller .

I see this error appears when running tests for subworkflows. Could be that we need to add a name to environment.yml? If two modules in a subworkflow have exactly the same environment.yml I suspect the environment names may clash

While this is not a subworkflow, the nf-test for this module is chained with other modules that use the exact same environment.yml. I am however not sure what the solution is.

@famosab
Copy link
Contributor

famosab commented Nov 1, 2024

In theory that should not happen with nf-test anymore as noted down here: #5836

They seemed to have thought about a fix: #6664

Maybe it is worth asking in the slack since this issue came up a few times :)

@Jorisvansteenbrugge Jorisvansteenbrugge removed the help wanted Extra attention is needed label Nov 11, 2024
@Jorisvansteenbrugge
Copy link
Contributor Author

Jorisvansteenbrugge commented Nov 11, 2024 •
edited
Loading

@famosab Thanks :)

It seems like we are waiting for nextflow-io/nextflow#5489 to get merged. Which should fix the issue (nextflow-io/nextflow#5485)

LouisLeNezet
LouisLeNezet requested changes Nov 29, 2024
View reviewed changes
Copy link
Contributor

@LouisLeNezet LouisLeNezet left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution.
I'm just a bit concerned on how the output is given back.
Is there a need for it to be in a zip format ?

modules/nf-core/pypgx/runngspipeline/main.nf Show resolved Hide resolved
modules/nf-core/pypgx/runngspipeline/main.nf Outdated Show resolved Hide resolved
modules/nf-core/pypgx/runngspipeline/meta.yml Show resolved Hide resolved
modules/nf-core/pypgx/runngspipeline/meta.yml Show resolved Hide resolved
modules/nf-core/pypgx/runngspipeline/meta.yml Show resolved Hide resolved
modules/nf-core/pypgx/runngspipeline/meta.yml Outdated Show resolved Hide resolved
modules/nf-core/pypgx/runngspipeline/main.nf Outdated Show resolved Hide resolved
modules/nf-core/pypgx/runngspipeline/meta.yml Outdated Show resolved Hide resolved
modules/nf-core/pypgx/runngspipeline/tests/main.nf.test Show resolved Hide resolved
@Jorisvansteenbrugge
Copy link
Contributor Author

Jorisvansteenbrugge commented Jan 10, 2025 •
edited
Loading

Thanks for the contribution. I'm just a bit concerned on how the output is given back. Is there a need for it to be in a zip format ?

@LouisLeNezet
The zip files are simply what the tool outputs, it is not something I added myself

@LouisLeNezet
Copy link
Contributor

Thanks for the contribution. I'm just a bit concerned on how the output is given back. Is there a need for it to be in a zip format ?

@LouisLeNezet The zip files are simply what the tool outputs, it is not something I added myself

That's fine for me then.

LouisLeNezet
LouisLeNezet approved these changes Jan 10, 2025
View reviewed changes
Copy link
Contributor

@LouisLeNezet LouisLeNezet left a comment

Choose a reason for hiding this comment

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

From looking at the other modules of this tool, this is ready to merge !
LGTM

@Jorisvansteenbrugge Jorisvansteenbrugge added this pull request to the merge queue Jan 13, 2025
Merged via the queue into master with commit e06e3b1 Jan 13, 2025
33 checks passed
@Jorisvansteenbrugge Jorisvansteenbrugge deleted the pypgx/runngspipeline branch January 13, 2025 07:54
mazzalab pushed a commit to mazzalab/nf-core_modules that referenced this pull request Jan 18, 2025
@Jorisvansteenbrugge @SPPearce @mazzalab
pypgx/runngspipeline (nf-core#6823)
1da34ed 
* pypgx/runngspipeline

* update PR comments

---------

Co-authored-by: Jorisvansteenbrugge <7196110+Jorisvansteenbrugge@users.noreply.github.com>
Co-authored-by: Simon Pearce <24893913+SPPearce@users.noreply.github.com>
github-merge-queue bot pushed a commit that referenced this pull request Jan 18, 2025
@mazzalab @Jorisvansteenbrugge
@SPPearce @renovate @vagkaratzas @prototaxites @nschcolnicov @GallVp
Bump wipertools to 1.1.4 (#7328)
e7f7c0f 
* Refactor fastq and report variable names for consistency across wipertools modules

* Refactor variable names for consistency in fastq and report processing modules

* Update test assertions to reflect changes in output variable names in fastqwiper module

* Update test assertions to match changes in report output structure in fastqwiper tests

* pypgx/runngspipeline (#6823)

* pypgx/runngspipeline

* update PR comments

---------

Co-authored-by: Jorisvansteenbrugge <7196110+Jorisvansteenbrugge@users.noreply.github.com>
Co-authored-by: Simon Pearce <24893913+SPPearce@users.noreply.github.com>

* Update actions/upload-artifact digest to 65c4c4a (#7293)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* fixed typo that produces "unknown recognition error type: groovyjarja… (#7295)

fixed typo that produces "unknown recognition error type: groovyjarjarantlr4.v4.runtime.LexerNoViableAltException" in pipelines

* Update Galah & add nf-tests (#7291)

* Update Galah & add nf-tests

* Fix linting

* Update main.nf

* Update inputs in meta.yml

* Fix missing snapshot output from broken test

* Added nftests to gseagsea (#7266)

* Added nftests to gseagsea

* Update input files

* fix nftests

* Address PR comments

* Add seed setting parameter for stabilizing results

* Fix linting

* Add default random seed

* Update datasets paths

---------

Co-authored-by: Simon Pearce <24893913+SPPearce@users.noreply.github.com>

* Bumped lima to 2.12.0 (#7294)

* Update container version to 1.1.4 and modify test names for clarity

* Remove obsolete test cases from fastqscatter and clean up pytest module configuration

* Update wipertools version to 1.1.4 in environment.yml files for fastqgather, fastqscatter, fastqwiper, and reportgather

* Update timestamps in fastqscatter test snapshots for consistency

---------

Co-authored-by: Joris van Steenbrugge <Jorisvansteenbrugge@users.noreply.github.com>
Co-authored-by: Jorisvansteenbrugge <7196110+Jorisvansteenbrugge@users.noreply.github.com>
Co-authored-by: Simon Pearce <24893913+SPPearce@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com>
Co-authored-by: Jim Downie <19718667+prototaxites@users.noreply.github.com>
Co-authored-by: Nicolás Schcolnicov <90359308+nschcolnicov@users.noreply.github.com>
Co-authored-by: Usman Rashid <usman@smme.edu.pk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LouisLeNezet LouisLeNezet LouisLeNezet approved these changes

Assignees
No one assigned
Labels
new module Adding a new module
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

new module: pypgx/runngspipeline
4 participants
@Jorisvansteenbrugge @famosab @LouisLeNezet @SPPearce