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

Added nf-test for cnvpytor/partition #6513

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Added nf-test for cnvpytor/partition #6513

wants to merge 2 commits into from

Conversation

GallVp
Copy link
Member

@GallVp GallVp commented Sep 1, 2024

Added nf-test for cnvpytor/partition with #6226

SPPearce
SPPearce requested changes Sep 3, 2024
View reviewed changes
Copy link
Contributor

@SPPearce SPPearce left a comment

Choose a reason for hiding this comment

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

Think this module needs some looking at, not sure how functional it really is.
Be worth checking if we can get prefix working too, rather than relying on pytor.baseName.

modules/nf-core/cnvpytor/callcnvs/main.nf
@@ -23,7 +23,7 @@ process CNVPYTOR_CALLCNVS {
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

This has no args.

modules/nf-core/cnvpytor/partition/tests/main.nf.test
[ id:'test'], // meta map
file(params.modules_testdata_base_path + 'genomics/homo_sapiens/illumina/pytor/test.pytor', checkIfExists: true)
]
input[1] = []
Copy link
Contributor

Choose a reason for hiding this comment

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

Think we need a test checking that this works, given that it didn't previously.

modules/nf-core/cnvpytor/partition/tests/nextflow.config Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't think this is doing anything as args is missing, and suspect it should be in the value channel anyway.

modules/nf-core/cnvpytor/partition/main.nf
@@ -19,15 +19,15 @@ process CNVPYTOR_PARTITION {
task.ext.when == null || task.ext.when

Copy link
Contributor

Choose a reason for hiding this comment

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

No args here either.

@SPPearce SPPearce added the help wanted Extra attention is needed label Sep 3, 2024
@GallVp GallVp marked this pull request as draft September 4, 2024 02:52
@GallVp
Copy link
Member Author

GallVp commented Sep 4, 2024

Thank you @SPPearce

This one needs some serious fixing. The test data produces empty outputs.

cnvpytor -root test.dup.pytor -partition 100 1000 10000

Currently the module is not creating any output. It is simply passing the input as output.

@LouisLeNezet LouisLeNezet linked an issue Feb 28, 2025 that may be closed by this pull request
nf-test migration: cnvpytor/partition #7552
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SPPearce SPPearce SPPearce requested changes

@sima-r sima-r Awaiting requested review from sima-r

@ramprasadn ramprasadn Awaiting requested review from ramprasadn

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
help wanted Extra attention is needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

nf-test migration: cnvpytor/partition
2 participants
@GallVp @SPPearce