Skip to content

Add primerprospector_analyzeprimers#11615

Open
vagkaratzas wants to merge 7 commits into
masterfrom
add-primerprospector_analyzeprimers
Open

Add primerprospector_analyzeprimers#11615
vagkaratzas wants to merge 7 commits into
masterfrom
add-primerprospector_analyzeprimers

Conversation

@vagkaratzas
Copy link
Copy Markdown
Contributor

@vagkaratzas vagkaratzas commented May 12, 2026

Workaround shim in main:

Primer Prospector 1.0.1 (this is an old codebase) is a Python 2 package and the BioContainer entrypoint uses Python 2.7.
The bug is specifically Python 2 behavior: range() requires integer arguments, and the package passes a
numpy.float64 during plotting.

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

Copy link
Copy Markdown

@timrozday-mgnify timrozday-mgnify left a comment

Choose a reason for hiding this comment

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

Thanks @vagkaratzas ! This looks useful.

I have one major issue with this module relating to incorrect handling of output hit files when there are multiple primers in the input primer file (a common case). The tests should be amended with this case.

I think it would be useful to support gzipped fasta inputs, but I don't know what nf-core good practice says about that.

-f "${fasta_arg}" \\
${primer_arg}

mv *_hits.txt "${prefix}_hits.txt"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

For multiple primers in the input primer file this will cause an error or overwrite some results. Rather all the files should be emitted in the hits output channel. If prefix renaming is desired then it needs to be performed on each of the produced hits files.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants