plastid metagene_generate, make_wiggle, psite #9482
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pinin4fjords
requested changes
Dec 5, 2025
Member
pinin4fjords
left a comment
pinin4fjords
left a comment
There was a problem hiding this comment.
Left some comments. In general:
- Only use channel inputs for the absolute tool-won't-run-without-them value arguments. Others should come via ext.args.
- Use metas for all file inputs.
- Don't manipulate the outputs of tools. Modules shouldn't be opinionated about that stuff.
- Versions outputs are being transitioned to topics as we speak. You could avoid getting caught in the crossfire by doing that now.
Contributor
Author
|
Thanks for the comprehensive feedback! I will address the points and change the code accordingly. |
Contributor
Author
|
Thanks again for your feedback. All issues should be resolved now. Let me know if you would like any further changes. |
pinin4fjords
reviewed
Dec 9, 2025
- Add mapping_rule val input (enum: fiveprime, threeprime, center, fiveprime_variable) - Move output_format to ext.args (optional arg per nf-core standards) - Add validation: error if p_offsets missing with fiveprime_variable - Remove hardcoded --fiveprime_variable - Update meta.yml with mapping_rule input and enum - Update tests with mapping_rule input 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
plastid/make_wiggle: nf-core standards compliance
- Consolidate multiple snapshot assertions into single snapshots per test - Remove snapshots of empty stub files (just check existence) - Exclude non-reproducible PNG from psite snapshots (matplotlib drift) - Format metagene_generate command across multiple lines 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Member
|
Hi @suhrig, I've opened another PR to your branch with suggestions to fix a CI reproducibility issue: suhrig#2 The psite PNG has non-reproducible md5sums due to matplotlib rendering differences across conda builds, so I've excluded it from the snapshot (just checking existence instead). Also tidied up the test assertions while I was at it - consolidated snapshots and removed snapshots of empty stub files. |
plastid: consolidate test snapshots and fix reproducibility
Member
|
@suhrig sorry, maybe I added some things to the snapshotting that shouldn't be there, let me fix it up |
Wig files have non-reproducible md5sums across environments.
Content is already validated via getText().contains('track').
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
metagene_profiles.txt and p_offsets.txt have non-reproducible md5sums. Content is already validated via getText().contains() checks. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
pinin4fjords
approved these changes
Dec 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds plastid with three submodules:
These functions are required to compute p-sites from Ribo-seq data.
I have also added test data here: nf-core/test-datasets#1809
PR checklist
versions.ymlfile.labelnf-core modules test <MODULE> --profile dockernf-core modules test <MODULE> --profile singularitynf-core modules test <MODULE> --profile conda