Skip to content

Conversation

@jonasscheid
Copy link
Contributor

@jonasscheid jonasscheid commented Jan 12, 2026

Linter seems to fail for topic channels currently. Tried to implement suggestion from Slack: https://nfcore.slack.com/archives/C09LJTQQ3EY/p1763134358185619?thread_ts=1762763322.812579&cid=C09LJTQQ3EY

Versions are captured properly in snapshot, though.

PR checklist

Closes #XXX

  • 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
  • If necessary, include test data in your PR.
  • Remove all TODO statements.
  • Broadcast software version numbers to topic: versions - See version_topics
  • 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
      • nf-core modules test <MODULE> --profile singularity
      • nf-core modules test <MODULE> --profile conda
    • For subworkflows:
      • nf-core subworkflows test <SUBWORKFLOW> --profile docker
      • nf-core subworkflows test <SUBWORKFLOW> --profile singularity
      • nf-core subworkflows test <SUBWORKFLOW> --profile conda

@mashehu
Copy link
Contributor

mashehu commented Jan 12, 2026

we haven't gotten the anchor syntax to work properly yet, so for now you need to duplicate the entries. I reran everything with the most recent dev version of tools and lint --fix, which looks like to fix all cases correctly, except for the thirdparty one...

@mashehu
Copy link
Contributor

mashehu commented Jan 12, 2026

I hope it was okay, that i pushed some fixes directly to your PR.
Could you update the snapshots to the new emit channel names?

@jonasscheid
Copy link
Contributor Author

Sure thing, thanks for fixing it @mashehu!

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.

We have a proceedure for deprecating modules:
https://nf-co.re/docs/guidelines/components/module_deprecation

@jonasscheid
Copy link
Contributor Author

@nf-core-bot fix linting

@jonasscheid
Copy link
Contributor Author

jonasscheid commented Jan 13, 2026

Thanks for the hint @SPPearce! No matter what I do, I cannot get the linter to pass the cometadapter module.

@mashehu
Copy link
Contributor

mashehu commented Jan 13, 2026

I'll have another look.

@jonasscheid
Copy link
Contributor Author

I'm puzzled. I did the --fix option as well as tried out other things as well with nf-core 3.5.1 linter. I had the same solution afaik but locally I got the linting error. Anyways, looks like it works now, thanks!

@mashehu
Copy link
Contributor

mashehu commented Jan 13, 2026

there are some fixes in the dev version of tools (updated yesterday), that is also the version CI uses in modules (topics are still quite hot and we keep finding edge cases)

@jonasscheid
Copy link
Contributor Author

Alright, thanks again! Looks like we are ready to review here :)

output:
tuple val(meta), path("*.fasta"), emit: decoy_fasta
path "versions.yml" , emit: versions
tuple val("${task.process}"), val('openms'), eval("FileInfo --help 2>&1 | grep -E '^Version' | sed 's/^.*Version: //; s/-.*\$//' | sed 's/ -*//; s/ .*\$//'"), emit: versions_openms, topic: versions
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh this is really really long, I'm sure AI could make it much shorter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried already a full Gemini3 day on version parsing 😂😂

Co-authored-by: Simon Pearce <24893913+SPPearce@users.noreply.github.com>
@jonasscheid
Copy link
Contributor Author

Can we merge? 😁

Co-authored-by: Simon Pearce <24893913+SPPearce@users.noreply.github.com>
@jonasscheid
Copy link
Contributor Author

Is your local nf-core lint from 3.5.1? I guess the dev branch used during CI is the reason why it works for you and not during CI, correct?

@SPPearce
Copy link
Contributor

The linting is complaining because the meta.yml hasn't been updated to match

@SPPearce
Copy link
Contributor

(the singularity errors are unrelated, need to update branch to get rid of them)

- openms:
type: string
description: The tool name
- "FileInfo --help 2>&1 | grep -E '^Version' | sed 's/^.*Version: //; s/-.*\\$//' | sed 's/ -*//; s/ .*\\$//'":
Copy link
Contributor Author

@jonasscheid jonasscheid Jan 15, 2026

Choose a reason for hiding this comment

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

Suggested change
- "FileInfo --help 2>&1 | grep -E '^Version' | sed 's/^.*Version: //; s/-.*\\$//' | sed 's/ -*//; s/ .*\\$//'":
- "FileInfo --help 2>&1 | sed -nE 's/^Version: ([0-9.]+).*/\\1/p'":

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