Skip to content

fix: use proper callable when accessing rule proxy with associated path modifier#3763

Merged
johanneskoester merged 2 commits intomainfrom
fix/rule-proxy-callable
Oct 2, 2025
Merged

fix: use proper callable when accessing rule proxy with associated path modifier#3763
johanneskoester merged 2 commits intomainfrom
fix/rule-proxy-callable

Conversation

@johanneskoester
Copy link
Copy Markdown
Contributor

@johanneskoester johanneskoester commented Oct 1, 2025

Description

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
    • Ensures path modifiers are correctly applied when callable inputs are wrapped as file-like annotations, producing correct input paths.
    • Preserves support for unwrapped callable inputs and aligns behavior between wrapped and unwrapped callables.
    • Reduces unexpected path mismatches in complex workflows, improving input resolution reliability.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 1, 2025

📝 Walkthrough

Walkthrough

Handles callables wrapped in _IOFile or AnnotatedString by extracting the underlying callable and applying the existing path modifier to the callable's result via an inner(wildcards) indirection instead of calling the wrapper directly.

Changes

Cohort / File(s) Summary
Rule input path modifier handling
src/snakemake/rules.py
When an input item is a callable wrapped in \_IOFile or AnnotatedString, extract the underlying callable (e.g., item._file.callable or item.callable), define inner(wildcards) to invoke it, and apply the path modifier to the callable's result rather than calling the wrapper directly. Retains support for non-wrapped callables.

Sequence Diagram(s)

sequenceDiagram
  participant Rule as Rule
  participant PM as PathModifier
  participant Wrapper as _IOFile / AnnotatedString
  participant UF as Underlying Callable

  Note over PM,Wrapper: New flow for callable inputs wrapped by helpers

  Rule->>PM: register input with modifier(item)
  PM->>Wrapper: detect wrapper contains callable
  alt wrapper contains callable
    PM->>Wrapper: extract callable (item._file.callable or item.callable)
    PM->>UF: call inner(wildcards) -> UF(wildcards)
    UF-->>PM: path(s)
    PM-->>Rule: apply modifier(path(s))
  else not wrapped callable
    PM-->>Rule: apply modifier(item(wildcards) or item)
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description retains placeholder text in the Description section and lacks any summary of the change, and the QC section shows the test case checkbox left unchecked, indicating missing confirmation of tests. This fails to meet the required template’s directive to add a description of the PR and provide completion of checklist items. Please replace the placeholder in the Description section with a clear summary of the changes made and ensure the QC checklist is completed by adding or referencing a relevant test case before merging.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly describes the primary fix implemented in the pull request by indicating that the proper callable is used when accessing a rule proxy with an associated path modifier, aligning closely with the summary of changes. It highlights the key change without extraneous details and is clear to a teammate reviewing history.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/rule-proxy-callable

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e35fab9 and 0af0d20.

📒 Files selected for processing (1)
  • src/snakemake/rules.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py

⚙️ CodeRabbit configuration file

**/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of the self argument of methods.
Do not suggest type annotation of the cls argument of classmethods.
Do not suggest return type annotation if a function or method does not contain a return statement.

Files:

  • src/snakemake/rules.py
🧬 Code graph analysis (1)
src/snakemake/rules.py (1)
src/snakemake/io/__init__.py (2)
  • _IOFile (259-931)
  • AnnotatedString (934-954)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (41)
  • GitHub Check: tests (9, ubuntu-latest, py313)
  • GitHub Check: tests (5, ubuntu-latest, py311)
  • GitHub Check: tests (10, ubuntu-latest, py313)
  • GitHub Check: tests (9, ubuntu-latest, py312)
  • GitHub Check: tests (10, ubuntu-latest, py311)
  • GitHub Check: tests (10, ubuntu-latest, py312)
  • GitHub Check: tests (8, ubuntu-latest, py312)
  • GitHub Check: tests (10, windows-2022, py313)
  • GitHub Check: tests (8, ubuntu-latest, py311)
  • GitHub Check: tests (9, ubuntu-latest, py311)
  • GitHub Check: tests (9, windows-2022, py313)
  • GitHub Check: tests (8, windows-2022, py313)
  • GitHub Check: tests (6, ubuntu-latest, py313)
  • GitHub Check: tests (6, ubuntu-latest, py312)
  • GitHub Check: tests (5, ubuntu-latest, py312)
  • GitHub Check: tests (7, ubuntu-latest, py313)
  • GitHub Check: tests (7, windows-2022, py313)
  • GitHub Check: tests (5, ubuntu-latest, py313)
  • GitHub Check: tests (8, ubuntu-latest, py313)
  • GitHub Check: tests (7, ubuntu-latest, py311)
  • GitHub Check: tests (6, windows-2022, py313)
  • GitHub Check: tests (6, ubuntu-latest, py311)
  • GitHub Check: tests (7, ubuntu-latest, py312)
  • GitHub Check: tests (4, windows-2022, py313)
  • GitHub Check: tests (3, windows-2022, py313)
  • GitHub Check: tests (5, windows-2022, py313)
  • GitHub Check: tests (4, ubuntu-latest, py313)
  • GitHub Check: tests (2, ubuntu-latest, py313)
  • GitHub Check: tests (2, ubuntu-latest, py311)
  • GitHub Check: tests (2, macos-latest, py313)
  • GitHub Check: tests (4, ubuntu-latest, py312)
  • GitHub Check: tests (2, windows-2022, py313)
  • GitHub Check: tests (3, ubuntu-latest, py313)
  • GitHub Check: tests (3, ubuntu-latest, py312)
  • GitHub Check: tests (4, ubuntu-latest, py311)
  • GitHub Check: tests (2, ubuntu-latest, py312)
  • GitHub Check: tests (3, ubuntu-latest, py311)
  • GitHub Check: tests (1, ubuntu-latest, py313)
  • GitHub Check: tests (1, ubuntu-latest, py311)
  • GitHub Check: tests (1, windows-2022, py313)
  • GitHub Check: tests (1, ubuntu-latest, py312)

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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f1517e8 and e35fab9.

📒 Files selected for processing (1)
  • src/snakemake/rules.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py

⚙️ CodeRabbit configuration file

**/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of the self argument of methods.
Do not suggest type annotation of the cls argument of classmethods.
Do not suggest return type annotation if a function or method does not contain a return statement.

Files:

  • src/snakemake/rules.py
🧬 Code graph analysis (1)
src/snakemake/rules.py (1)
src/snakemake/io/__init__.py (4)
  • _IOFile (259-931)
  • is_callable (310-311)
  • is_callable (945-946)
  • is_callable (1163-1168)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (45)
  • GitHub Check: tests (10, ubuntu-latest, py313)
  • GitHub Check: tests (10, ubuntu-latest, py312)
  • GitHub Check: tests (10, windows-2022, py313)
  • GitHub Check: tests (9, windows-2022, py313)
  • GitHub Check: tests (10, ubuntu-latest, py311)
  • GitHub Check: tests (9, ubuntu-latest, py312)
  • GitHub Check: tests (5, windows-2022, py313)
  • GitHub Check: tests (9, ubuntu-latest, py313)
  • GitHub Check: tests (7, ubuntu-latest, py312)
  • GitHub Check: tests (9, ubuntu-latest, py311)
  • GitHub Check: tests (7, windows-2022, py313)
  • GitHub Check: tests (8, windows-2022, py313)
  • GitHub Check: tests (8, ubuntu-latest, py313)
  • GitHub Check: tests (6, windows-2022, py313)
  • GitHub Check: tests (6, ubuntu-latest, py311)
  • GitHub Check: tests (8, ubuntu-latest, py312)
  • GitHub Check: tests (8, ubuntu-latest, py311)
  • GitHub Check: tests (7, ubuntu-latest, py313)
  • GitHub Check: tests (7, ubuntu-latest, py311)
  • GitHub Check: tests (6, ubuntu-latest, py312)
  • GitHub Check: tests (6, macos-latest, py313)
  • GitHub Check: tests (5, macos-latest, py313)
  • GitHub Check: tests (5, ubuntu-latest, py312)
  • GitHub Check: tests (5, ubuntu-latest, py313)
  • GitHub Check: tests (6, ubuntu-latest, py313)
  • GitHub Check: tests (5, ubuntu-latest, py311)
  • GitHub Check: tests (4, windows-2022, py313)
  • GitHub Check: tests (3, windows-2022, py313)
  • GitHub Check: tests (4, ubuntu-latest, py312)
  • GitHub Check: tests (3, macos-latest, py313)
  • GitHub Check: tests (4, ubuntu-latest, py313)
  • GitHub Check: tests (4, ubuntu-latest, py311)
  • GitHub Check: tests (3, ubuntu-latest, py313)
  • GitHub Check: tests (1, windows-2022, py313)
  • GitHub Check: tests (2, ubuntu-latest, py312)
  • GitHub Check: tests (3, ubuntu-latest, py312)
  • GitHub Check: tests (3, ubuntu-latest, py311)
  • GitHub Check: tests (2, ubuntu-latest, py311)
  • GitHub Check: tests (2, ubuntu-latest, py313)
  • GitHub Check: tests (2, windows-2022, py313)
  • GitHub Check: tests (1, macos-latest, py313)
  • GitHub Check: tests (2, macos-latest, py313)
  • GitHub Check: tests (1, ubuntu-latest, py312)
  • GitHub Check: tests (1, ubuntu-latest, py311)
  • GitHub Check: tests (1, ubuntu-latest, py313)

Comment thread src/snakemake/rules.py Outdated
@johanneskoester johanneskoester merged commit b5d12c8 into main Oct 2, 2025
109 of 113 checks passed
@johanneskoester johanneskoester deleted the fix/rule-proxy-callable branch October 2, 2025 09:09
johanneskoester pushed a commit that referenced this pull request Oct 2, 2025
🤖 I have created a release *beep* *boop*
---


##
[9.11.8](v9.11.7...v9.11.8)
(2025-10-02)


### Bug Fixes

* use proper callable when accessing rule proxy with associated path
modifier ([#3763](#3763))
([b5d12c8](b5d12c8))


### Documentation

* add snakemake logo to automatic mastodon release posts
([#3764](#3764))
([7b66250](7b66250))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
kjohnsen pushed a commit to kjohnsen/snakemake that referenced this pull request Dec 15, 2025
…th modifier (snakemake#3763)

### Description

<!--Add a description of your PR here-->

### QC
<!-- Make sure that you can tick the boxes below. -->

* [ ] 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**
* Ensures path modifiers are correctly applied when callable inputs are
wrapped as file-like annotations, producing correct input paths.
* Preserves support for unwrapped callable inputs and aligns behavior
between wrapped and unwrapped callables.
* Reduces unexpected path mismatches in complex workflows, improving
input resolution reliability.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
kjohnsen pushed a commit to kjohnsen/snakemake that referenced this pull request Dec 15, 2025
🤖 I have created a release *beep* *boop*
---


##
[9.11.8](snakemake/snakemake@v9.11.7...v9.11.8)
(2025-10-02)


### Bug Fixes

* use proper callable when accessing rule proxy with associated path
modifier ([snakemake#3763](snakemake#3763))
([b5d12c8](snakemake@b5d12c8))


### Documentation

* add snakemake logo to automatic mastodon release posts
([snakemake#3764](snakemake#3764))
([7b66250](snakemake@7b66250))

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

1 participant