Skip to content

fix: issue 3642#4054

Merged
johanneskoester merged 4 commits intosnakemake:mainfrom
ccondon894:fix-issue-3642
Mar 11, 2026
Merged

fix: issue 3642#4054
johanneskoester merged 4 commits intosnakemake:mainfrom
ccondon894:fix-issue-3642

Conversation

@ccondon894
Copy link
Copy Markdown
Contributor

@ccondon894 ccondon894 commented Mar 11, 2026

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

QC

  • The PR contains a test case for the changes or the changes are already covered by an existing test case.
  • 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).

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.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5e7524ca-157d-4367-a31a-4a00b3b3888f

📥 Commits

Reviewing files that changed from the base of the PR and between 751ed99 and 1f12029.

📒 Files selected for processing (1)
  • tests/tests.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/tests.py

📝 Walkthrough

Walkthrough

Parse CLI argv to recognize an explicit target separator (--) when using profiles: split argv at --, parse the pre-separator portion (including --profile), then append trailing items as explicit parsed_args.targets. Adds tests and fixtures validating this behavior.

Changes

Cohort / File(s) Summary
CLI argument parsing
src/snakemake/cli.py
Detect -- in argv; when present, split into pre- and post-separator slices, parse the pre-separator args (so profiles are applied), then append post-separator items to parsed_args.targets. Preserves prior behavior when -- absent.
Test workflow
tests/test_profile_double_dash/Snakefile
New Snakefile with rules all, one, and two to exercise target selection with profiles and explicit -- targets.
Test fixtures
tests/test_profile_double_dash/profile/config.yaml, tests/test_profile_double_dash/expected-results/done.txt
Add profile config setting default-resources.runtime: 10 and expected output done.txt containing done.
Test function
tests/tests.py
Add test_profile_double_dash() (skipped on Windows) that runs Snakemake with --profile and -- done.txt to verify trailing targets are correctly recognized alongside profiles.

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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'fix: issue 3642' is vague and does not clearly describe the main change; it only references an issue number without explaining what was fixed. Revise the title to be more descriptive, such as 'Fix profile argument handling with explicit targets using --' to better convey the change.
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The PR description clearly explains the problem, solution, and includes test case confirmation; all required sections are addressed and QC checkboxes are properly marked.
Linked Issues check ✅ Passed The PR directly addresses issue #3642 by fixing the argument parsing flow to prevent profile-supplied config arguments from being placed after '--' and misinterpreted as targets.
Out of Scope Changes check ✅ Passed All changes are scoped to the issue: argument parsing fix in cli.py, test files for profile double-dash handling, and test case addition.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between 97d74ba and 57e8eff.

📒 Files selected for processing (5)
  • src/snakemake/cli.py
  • tests/test_profile_double_dash/Snakefile
  • tests/test_profile_double_dash/expected-results/done.txt
  • tests/test_profile_double_dash/profile/config.yaml
  • tests/tests.py

@ccondon894 ccondon894 changed the title Fix issue 3642 fix: issue 3642 Mar 11, 2026
@johanneskoester johanneskoester merged commit 76e6fc2 into snakemake:main Mar 11, 2026
112 of 113 checks passed
LKress pushed a commit to LKress/snakemake that referenced this pull request Mar 11, 2026
<!--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>
johanneskoester pushed a commit that referenced this pull request Mar 13, 2026
🤖 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).
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.

profile clashes with -- argument separator

2 participants