Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughParse CLI argv to recognize an explicit target separator ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as "Snakemake CLI"
participant Parser as "configargparse\nParser"
participant Profile as "Profile\nLoader"
participant Core as "Snakemake\nCore"
User->>CLI: invoke with argv (includes `--` and targets)
CLI->>CLI: detect `--`, split argv_before and argv_after
CLI->>Parser: parse argv_before (includes `--profile`)
Parser->>Profile: request profile defaults
Profile-->>Parser: return profile defaults
Parser-->>CLI: return parsed_args
CLI->>CLI: append argv_after items to parsed_args.targets
CLI->>Core: invoke execution with parsed_args (targets included)
Core-->>User: run workflow producing expected targets
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/test_profile_double_dash/Snakefile`:
- Around line 1-3: The default target (rule all) currently lists "done.txt",
which masks the test’s intent; change rule all to depend on a different artifact
(e.g., "default.txt" or "all_done.txt") so that "done.txt" is not part of the
default DAG and only gets built when explicitly passed after the "--" separator.
Update the rule named "all" to reference the new default filename and ensure
"done.txt" is produced only by its specific rule so the test actually verifies
preservation of the post-separator target.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 86a7b4f7-ff74-430e-a332-54b8580aee43
📒 Files selected for processing (5)
src/snakemake/cli.pytests/test_profile_double_dash/Snakefiletests/test_profile_double_dash/expected-results/done.txttests/test_profile_double_dash/profile/config.yamltests/tests.py
<!--Add a description of your PR here--> When `--profile` is combined with `--` to specify explicit targets (ex: `snakemake --profile p -- target.txt`), configargparse appends profile config args to the end of argv, placing them after `--`. - Fix by splitting argv at `--` before the profile re-parse, parsing only the options portion, then manually restoring the explicit targets. - Added test `test_profile_double_dash` . Fixes snakemake#3642 ### QC <!-- Make sure that you can tick the boxes below. --> * [x] The PR contains a test case for the changes or the changes are already covered by an existing test case. * [x] The documentation (`docs/`) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Corrected handling of explicit targets provided after “--” when using profiles so trailing targets are properly recognized. * **Tests** * Added an automated test and accompanying workflow/example outputs to validate the target-separator behavior with profiles. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Chris C <chris@Chriss-MacBook-Pro-2.local>
🤖 I have created a release *beep* *boop* --- ## [9.17.0](v9.16.3...v9.17.0) (2026-03-13) ### Features * Allow storing snakemake metadata in files or databases ([#4012](#4012)) ([dd75f31](dd75f31)) * Allow to specify comparison command per-unit test ([#3956](#3956)) ([b88171c](b88171c)) * job table orderd topological when run is started ([#4018](#4018)) ([75cf506](75cf506)) * lambda functions for priority in rules ([#3253](#3253)) ([d2aa226](d2aa226)) * Make on... directive of modules accessible ([#4050](#4050)) ([e9f2e1c](e9f2e1c)) ### Bug Fixes * adjust conda tests to not fail on apple silicon; fix [#4040](#4040) ([#4049](#4049)) ([f5b0142](f5b0142)) * allow "--containerize apptainer" to output apptainer format instead of dockerfile ([#4030](#4030)) ([f5cac30](f5cac30)) * apptainer command not recognized when singularity is absent ([#4010](#4010)) ([b8162e2](b8162e2)) * capture stderr when tests fail ([#3995](#3995)) ([97d74ba](97d74ba)) * **docs:** make Data-dependent conditional execution a complete example ([#4043](#4043)) ([3a1d7f2](3a1d7f2)) * don't build the DAG when running unlock. Fixes [#4000](#4000) and [#198](#198) ([#4007](#4007)) ([acf79fd](acf79fd)) * Ensure pixi tasks may be run as advertised ([#4046](#4046)) ([88253c2](88253c2)) * fix checkpoint handling corner cases ([#3870](#3870) and [#3559](#3559)) ([#4015](#4015)) ([63f4257](63f4257)) * issue 3642 ([#4054](#4054)) ([76e6fc2](76e6fc2)) * issue 3815 ([#4026](#4026)) ([b0eec96](b0eec96)) * logging None in shellcmd context causes error ([#4064](#4064)) ([d0652cd](d0652cd)) * lookup function returns default value for empty DataFrame queries ([#4056](#4056)) ([f71de97](f71de97)) * make `cache: omit-software` a rule specific property ([#4085](#4085)) ([034a9e7](034a9e7)) * reduce number of tests leaving temporary files behind ([#4033](#4033)) ([a3a1c97](a3a1c97)) * regression in dynamic resource handling ([#4038](#4038)) ([f2c554a](f2c554a)) * somewhat shorter announce message ([#4080](#4080)) ([57efc71](57efc71)) ### Performance Improvements * switch reretry with tenacity; decouple container classes (with Python 3.7 compat for old scripts) from rest of the codebase (enabling moving to newer python versions) ([#4032](#4032)) ([ffb19e7](ffb19e7)) ### Documentation * Add AI-assisted contributions policy to contributing guidelines ([#4051](#4051)) ([dd70526](dd70526)) * **codebase:** Update & simplify plugin architecture section ([#4052](#4052)) ([176cf63](176cf63)) * Correct workflow.source_path() description in documentation ([#4036](#4036)) ([45883c5](45883c5)) * fixed wrong code example for collect() function ([#4037](#4037)) ([5c85ed8](5c85ed8)) * Minor docs improvements ([#4089](#4089)) ([29ea226](29ea226)) * switch to sphinx_design for tabs ([#3976](#3976)) ([9674614](9674614)) * typo in the migration table breaking a pip install command ([#4024](#4024)) ([66f9dda](66f9dda)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
When
--profileis combined with--to specify explicit targets (ex:snakemake --profile p -- target.txt), configargparse appends profile config args to the end of argv, placing them after--.--before the profile re-parse, parsing only the options portion, then manually restoring the explicit targets.test_profile_double_dash.Fixes #3642
QC
docs/) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).Summary by CodeRabbit