Skip to content

New module: octopusv/merge#11617

Merged
SPPearce merged 17 commits into
nf-core:masterfrom
manascripts:octopusv-merge
May 21, 2026
Merged

New module: octopusv/merge#11617
SPPearce merged 17 commits into
nf-core:masterfrom
manascripts:octopusv-merge

Conversation

@manascripts
Copy link
Copy Markdown
Member

PR checklist

Closes #11516

  • 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

Comment thread modules/nf-core/octopusv/merge/main.nf Outdated
Comment thread modules/nf-core/octopusv/merge/main.nf Outdated
@manascripts manascripts requested a review from SPPearce May 19, 2026 14:15
Comment thread modules/nf-core/octopusv/merge/main.nf Outdated
Comment thread modules/nf-core/octopusv/merge/main.nf Outdated
Comment thread modules/nf-core/octopusv/merge/main.nf Outdated
Comment thread modules/nf-core/octopusv/merge/meta.yml Outdated
@SPPearce
Copy link
Copy Markdown
Contributor

You can put the valid list back into the module code if you like

@SPPearce SPPearce closed this May 19, 2026
auto-merge was automatically disabled May 19, 2026 15:48

Pull request was closed

@SPPearce
Copy link
Copy Markdown
Contributor

You can put the valid list back into the module code if you like

@SPPearce SPPearce reopened this May 19, 2026
@SPPearce
Copy link
Copy Markdown
Contributor

(sorry, just accidentally closed the PR!)

manascripts and others added 4 commits May 20, 2026 10:20
Co-authored-by: Simon Pearce <24893913+SPPearce@users.noreply.github.com>
Co-authored-by: Simon Pearce <24893913+SPPearce@users.noreply.github.com>
Co-authored-by: Simon Pearce <24893913+SPPearce@users.noreply.github.com>
Co-authored-by: Simon Pearce <24893913+SPPearce@users.noreply.github.com>
@manascripts
Copy link
Copy Markdown
Member Author

I was thinking of adding this: def strategy_flags = ['union', 'intersect', 'specific', 'min-support', 'exact-support', 'max-support', 'expression'] and if (!strategy_flags.contains(merge_strategy)) { error("OCTOPUSV_MERGE: invalid merge strategy '${merge_strategy}'. Allowed: ${strategy_flags.join(', ')}") }

But I think it is fine as is, since some flags also require an INT or a string after. For such cases one needs to refer to the tool documentation anyway. Do you think I should add this?

@manascripts manascripts requested a review from SPPearce May 20, 2026 11:47
Copy link
Copy Markdown
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.

Ok, particularly if some strategies require an extra value as well, validation isn't as simple.
I'm happy as is.

@manascripts
Copy link
Copy Markdown
Member Author

Cool thanks, I added an optional output (an upset plot) which I found later on. and bumped the version to 0.3.3 (new recipe came out only 8 hours ago).

@SPPearce SPPearce enabled auto-merge May 21, 2026 12:39
@SPPearce SPPearce added this pull request to the merge queue May 21, 2026
Merged via the queue into nf-core:master with commit d5584f1 May 21, 2026
25 checks passed
manascripts added a commit to manascripts/modules that referenced this pull request May 21, 2026
* Add octopusv/merge module

* test-data path  fix

* fix versioning

* add test data and update default merging logic

* update merge flag

* update tests

* nextflow lint fixes + test update

* Adress review feedback

* minor change in meta.yml

* Update modules/nf-core/octopusv/merge/meta.yml

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

* Update modules/nf-core/octopusv/merge/main.nf

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

* Update modules/nf-core/octopusv/merge/main.nf

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

* Update modules/nf-core/octopusv/merge/main.nf

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

* minor changes to test

* remove edam for bgzip, add note to strategy_flag (meta.yml)

* bump version + add upset plot output

---------

Co-authored-by: Simon Pearce <24893913+SPPearce@users.noreply.github.com>
@manascripts manascripts deleted the octopusv-merge branch May 21, 2026 13:06
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.

new module: octopusv/merge

2 participants