Skip to content

Update: pangolin modules#12211

Open
ammaraziz wants to merge 5 commits into
nf-core:masterfrom
ammaraziz:pangolin-update
Open

Update: pangolin modules#12211
ammaraziz wants to merge 5 commits into
nf-core:masterfrom
ammaraziz:pangolin-update

Conversation

@ammaraziz

Copy link
Copy Markdown
Contributor

Hi guys,

Retrying this pull request, last one had a silly issue on my end.

  1. Migrated to using topics channels for both modules

  2. Modified the test files as they were testing for version.yml

  3. Update dependencies:

    • pangolin version updated to 4.4
    • pangolin-data version updated to 1.39
    • snakemake-minimal version updated to 9.21.1 - I chose a version that looked stable, fixed bugs but didnt introduce too many new features just to be sure.

Thanks!

@LouisLeNezet LouisLeNezet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just some simplification on the version expression, but good to go otherwise.

@@ -63,7 +63,6 @@ nextflow_process {
{ assert path(process.out.report[0][1]).readLines().first().contains("taxon,lineage,conflict,ambiguity_score,scorpio_call,scorpio_support,scorpio_conflict,scorpio_notes,version,pangolin_version,scorpio_version,constellation_version,is_designated,qc_status,qc_notes,note") },
{ assert snapshot(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you instead use sanitizeOutput(process.out, unstableKeys:["report"]) ?
This way this will also snapshot the versions.

output:
tuple val(meta), path('*.csv'), emit: report
path "versions.yml" , emit: versions
tuple val("${task.process}"), val('pangolin'), eval("pangolin -v | sed -n 's/pangolin \\([0-9.]*\\).*/\\1/p'"), emit: versions_pangolin, topic: versions

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This could be simplified as

Suggested change
tuple val("${task.process}"), val('pangolin'), eval("pangolin -v | sed -n 's/pangolin \\([0-9.]*\\).*/\\1/p'"), emit: versions_pangolin, topic: versions
tuple val("${task.process}"), val('pangolin'), eval("pangolin -v | sed 's/pangolin //'"), emit: versions_pangolin, topic: versions

pattern: "*.{csv}"
ontologies:
- edam: http://edamontology.org/format_3752 # CSV
- edam: http://edamontology.org/format_3752

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
- edam: http://edamontology.org/format_3752
- edam: http://edamontology.org/format_3752 #CSV

@@ -25,7 +25,6 @@ nextflow_process {
{ assert process.success },
{ assert snapshot(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here

path("${prefix}") , emit: db
path "versions.yml" , emit: versions
path("${prefix}") , emit: db
tuple val("${task.process}"), val('pangolin-dataset'), eval("pangolin -pv | sed -n 's/pangolin-data \\([0-9.]*\\).*/\\1/p'"), emit: versions_pangolin_dataset, topic: versions

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here

Suggested change
tuple val("${task.process}"), val('pangolin-dataset'), eval("pangolin -pv | sed -n 's/pangolin-data \\([0-9.]*\\).*/\\1/p'"), emit: versions_pangolin_dataset, topic: versions
tuple val("${task.process}"), val('pangolin-dataset'), eval("pangolin -pv | sed 's/pangolin-data //'"), emit: versions_pangolin_dataset, topic: versions

@atrigila atrigila added the awaiting-changes will be closed after 30 days label Jul 2, 2026
@SPPearce

Copy link
Copy Markdown
Contributor

@ammaraziz , do you want to get this finished off?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-changes will be closed after 30 days

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants