Skip to content

fix: fix --consider-ancient for input functions (was not applied in those cases)#3788

Merged
johanneskoester merged 1 commit into
mainfrom
fix/mark_ancient_func
Oct 11, 2025
Merged

fix: fix --consider-ancient for input functions (was not applied in those cases)#3788
johanneskoester merged 1 commit into
mainfrom
fix/mark_ancient_func

Conversation

@johanneskoester

@johanneskoester johanneskoester commented Oct 11, 2025

Copy link
Copy Markdown
Contributor

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 the “ancient” flag is consistently applied to inputs and outputs after default flags are set.
    • Correctly propagates the “ancient” flag for both string and callable items, improving reliable rule evaluation and preventing unintended re-runs.

@coderabbitai

coderabbitai Bot commented Oct 11, 2025

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Adds conditional propagation of the "ancient" flag in rule item processing: after default flag application, both string and callable items receive the ancient flag when mark_ancient is true. No public API changes; no other control flow or error handling modifications.

Changes

Cohort / File(s) Change Summary
Flag propagation in rule items
src/snakemake/rules.py
After applying default flags, attach the ancient flag to string and callable items when mark_ancient is true. No other behavior changes.

Sequence Diagram(s)

sequenceDiagram
  participant R as Rule
  participant M as FlagMarker
  participant I as Item (string/callable)

  R->>M: process(item, mark_ancient)
  M->>I: apply default flags

  alt mark_ancient == true
    M->>I: attach "ancient" flag
  else mark_ancient == false
    M-->>I: no additional flag
  end

  I-->>R: item with updated flags
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The PR description still contains only placeholder text in the description section and lacks any actual summary of the changes or rationale, even though the QC checklist is completed. This does not fulfill the template requirement to provide a descriptive overview of the PR’s content. Replace the placeholder in the description section with a concise overview of what was changed, why the change was necessary, and how it was implemented to meet the repository’s template requirements.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly identifies the primary change by stating that the --consider-ancient flag is now applied to input functions, which directly matches the modifications described in the diff summary. Although it includes a redundant parenthetical, it remains specific and accurately reflects the main fix introduced by the PR.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/mark_ancient_func

📜 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 75d111d and 48ee97f.

📒 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 (1)
  • flag (980-990)
⏰ 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). (46)
  • GitHub Check: tests (10, ubuntu-latest, py312)
  • GitHub Check: tests (6, ubuntu-latest, py313)
  • GitHub Check: tests (10, ubuntu-latest, py313)
  • GitHub Check: tests (9, ubuntu-latest, py311)
  • GitHub Check: tests (4, ubuntu-latest, py312)
  • GitHub Check: tests (10, windows-2022, py313)
  • GitHub Check: tests (10, ubuntu-latest, py311)
  • GitHub Check: tests (9, ubuntu-latest, py312)
  • GitHub Check: tests (9, windows-2022, py313)
  • GitHub Check: tests (8, ubuntu-latest, py313)
  • GitHub Check: tests (9, ubuntu-latest, py313)
  • GitHub Check: tests (8, windows-2022, py313)
  • GitHub Check: tests (7, ubuntu-latest, py312)
  • GitHub Check: tests (8, ubuntu-latest, py312)
  • GitHub Check: tests (7, ubuntu-latest, py311)
  • GitHub Check: tests (6, ubuntu-latest, py312)
  • GitHub Check: tests (7, windows-2022, py313)
  • GitHub Check: tests (8, ubuntu-latest, py311)
  • GitHub Check: tests (7, ubuntu-latest, py313)
  • GitHub Check: tests (6, ubuntu-latest, py311)
  • GitHub Check: tests (4, ubuntu-latest, py311)
  • GitHub Check: tests (6, windows-2022, py313)
  • GitHub Check: tests (5, macos-latest, py313)
  • GitHub Check: tests (5, ubuntu-latest, py313)
  • GitHub Check: tests (5, windows-2022, py313)
  • GitHub Check: tests (5, ubuntu-latest, py312)
  • GitHub Check: tests (4, windows-2022, py313)
  • GitHub Check: tests (4, ubuntu-latest, py313)
  • GitHub Check: tests (3, macos-latest, py313)
  • GitHub Check: tests (4, macos-latest, py313)
  • GitHub Check: tests (5, ubuntu-latest, py311)
  • GitHub Check: tests (3, ubuntu-latest, py313)
  • GitHub Check: tests (2, ubuntu-latest, py311)
  • GitHub Check: tests (3, windows-2022, py313)
  • GitHub Check: tests (3, ubuntu-latest, py312)
  • GitHub Check: tests (1, ubuntu-latest, py313)
  • GitHub Check: tests (1, macos-latest, py313)
  • GitHub Check: tests (2, windows-2022, py313)
  • GitHub Check: tests (2, macos-latest, py313)
  • GitHub Check: tests (3, ubuntu-latest, py311)
  • GitHub Check: tests (1, ubuntu-latest, py312)
  • GitHub Check: tests (2, ubuntu-latest, py312)
  • GitHub Check: tests (1, ubuntu-latest, py311)
  • GitHub Check: tests (2, ubuntu-latest, py313)
  • GitHub Check: tests (1, windows-2022, py313)
  • GitHub Check: apidocs
🔇 Additional comments (1)
src/snakemake/rules.py (1)

581-582: LGTM! Ancient flag now correctly propagates to callable inputs.

The change mirrors the existing logic for string items (lines 543-544) and ensures that the --consider-ancient option works correctly for input functions. When a callable is evaluated in expand_input (lines 882-886), the ancient flag will be inherited by the resulting IOFile objects.


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.

@johanneskoester johanneskoester merged commit c28da7c into main Oct 11, 2025
60 checks passed
@johanneskoester johanneskoester deleted the fix/mark_ancient_func branch October 11, 2025 11:53
johanneskoester pushed a commit that referenced this pull request Oct 12, 2025
🤖 I have created a release *beep* *boop*
---


##
[9.13.2](v9.13.1...v9.13.2)
(2025-10-12)


### Bug Fixes

* fix --consider-ancient for input functions (was not applied in those
cases) ([#3788](#3788))
([c28da7c](c28da7c))
* only modify wrapper URI if imported rule actually uses a wrapper
([#3790](#3790))
([08ef3b4](08ef3b4))

---
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
…hose cases) (snakemake#3788)

### Description

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

### 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**
* Ensures the “ancient” flag is consistently applied to inputs and
outputs after default flags are set.
* Correctly propagates the “ancient” flag for both string and callable
items, improving reliable rule evaluation and preventing unintended
re-runs.

<!-- 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.13.2](snakemake/snakemake@v9.13.1...v9.13.2)
(2025-10-12)


### Bug Fixes

* fix --consider-ancient for input functions (was not applied in those
cases) ([snakemake#3788](snakemake#3788))
([c28da7c](snakemake@c28da7c))
* only modify wrapper URI if imported rule actually uses a wrapper
([snakemake#3790](snakemake#3790))
([08ef3b4](snakemake@08ef3b4))

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