Skip to content

fix: revert breaking change in 9.11.9 disallowing empty input files even when unused#3810

Merged
johanneskoester merged 1 commit intomainfrom
revert-emptyerror-breaking-change
Oct 30, 2025
Merged

fix: revert breaking change in 9.11.9 disallowing empty input files even when unused#3810
johanneskoester merged 1 commit intomainfrom
revert-emptyerror-breaking-change

Conversation

@Hocnonsense
Copy link
Copy Markdown
Contributor

@Hocnonsense Hocnonsense commented Oct 27, 2025

Reverts a breaking change introduced in version 9.11.9 where all input files were required to be non-empty, even if they were not actually used by a rule.

#3801 suggested that it would be acceptable to raise an error for this in the future, as long as there is a deprecation warning period or it comes with a major version bump, and a warning sounds good for now.

Thanks @corneliusroemer for pointing this out, and apologies for my earlier overzealous modification.

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
    • Improved handling of empty file paths by providing informative guidance instead of raising an error. The system now logs a warning message directing users to use an empty list ([]) to indicate no files, making workflows more forgiving and easier to debug.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 27, 2025

📝 Walkthrough

Walkthrough

The check() function in src/snakemake/io/__init__.py no longer raises a WorkflowError when encountering an empty file path string. Instead, it logs a warning message that includes context from the associated rule and guidance about using an empty list.

Changes

Cohort / File(s) Summary
Error handling modification
src/snakemake/io/__init__.py
Modified check() function to replace exception raising with warning logging when file path is an empty string; computes optional spec prefix from rule context for warning message

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Single file modification with straightforward logic change (exception → warning)
  • Localized behavior change in one conditional branch
  • No structural refactoring or multi-file coordination required

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description follows the required template structure and includes a clear explanation of the change being reverted, references to the related issue (#3801), and appropriate acknowledgments. However, the QC checklist section has both required items left unchecked without any explanation. The template explicitly instructs "Make sure that you can tick the boxes below," and both items—confirming test case coverage and verifying documentation updates—remain unchecked, leaving uncertainty about whether these requirements have been satisfied or are unnecessary for this change. The author should verify and complete the QC checklist by either checking the boxes to confirm the requirements are met (test case exists or is covered, documentation updated or not necessary) or adding brief explanatory notes if any items are not applicable. This ensures the description fully aligns with the repository's template requirements and clearly communicates the status of quality checks for the change.
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 pull request title "fix: revert breaking change in 9.11.9 disallowing empty input files even when unused" directly and accurately describes the main change in the PR. According to the raw_summary, the modification changes the check() function from raising a WorkflowError on empty file paths to logging a warning instead, which aligns perfectly with the title's description of reverting the breaking change. The title is concise, specific, and follows conventional formatting with the "fix:" prefix, making it clear to reviewers scanning the history.
✨ 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 revert-emptyerror-breaking-change

📜 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 f8b8620 and c38707d.

📒 Files selected for processing (1)
  • src/snakemake/io/__init__.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/io/__init__.py
🧬 Code graph analysis (1)
src/snakemake/io/__init__.py (2)
src/snakemake/workflow.py (3)
  • rule (1800-2068)
  • name (2336-2341)
  • snakefile (1576-1581)
src/snakemake/rules.py (4)
  • name (142-143)
  • name (146-147)
  • lineno (150-151)
  • snakefile (154-155)
⏰ 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 (9, ubuntu-latest, py311)
  • GitHub Check: tests (10, ubuntu-latest, py311)
  • GitHub Check: tests (10, ubuntu-latest, py312)
  • GitHub Check: tests (7, ubuntu-latest, py311)
  • GitHub Check: tests (10, ubuntu-latest, py313)
  • GitHub Check: tests (5, windows-2022, py313)
  • GitHub Check: tests (10, windows-2022, py313)
  • GitHub Check: tests (7, windows-2022, py313)
  • GitHub Check: tests (8, ubuntu-latest, py313)
  • GitHub Check: tests (9, windows-2022, py313)
  • GitHub Check: tests (9, ubuntu-latest, py312)
  • GitHub Check: tests (8, ubuntu-latest, py312)
  • GitHub Check: tests (9, ubuntu-latest, py313)
  • GitHub Check: tests (7, ubuntu-latest, py313)
  • GitHub Check: tests (8, ubuntu-latest, py311)
  • GitHub Check: tests (7, ubuntu-latest, py312)
  • GitHub Check: tests (6, windows-2022, py313)
  • GitHub Check: tests (8, macos-latest, py313)
  • GitHub Check: tests (5, ubuntu-latest, py311)
  • GitHub Check: tests (8, windows-2022, py313)
  • GitHub Check: tests (4, ubuntu-latest, py313)
  • GitHub Check: tests (6, ubuntu-latest, py312)
  • GitHub Check: tests (6, ubuntu-latest, py313)
  • GitHub Check: tests (4, macos-latest, py313)
  • GitHub Check: tests (3, macos-latest, py313)
  • GitHub Check: tests (6, ubuntu-latest, py311)
  • GitHub Check: tests (4, ubuntu-latest, py311)
  • GitHub Check: tests (3, windows-2022, py313)
  • GitHub Check: tests (4, windows-2022, py313)
  • GitHub Check: tests (5, ubuntu-latest, py312)
  • GitHub Check: tests (3, ubuntu-latest, py313)
  • GitHub Check: tests (5, ubuntu-latest, py313)
  • GitHub Check: tests (4, ubuntu-latest, py312)
  • GitHub Check: tests (2, ubuntu-latest, py312)
  • GitHub Check: tests (3, ubuntu-latest, py312)
  • GitHub Check: tests (2, macos-latest, py313)
  • GitHub Check: tests (3, ubuntu-latest, py311)
  • GitHub Check: tests (2, windows-2022, py313)
  • GitHub Check: tests (2, ubuntu-latest, py313)
  • GitHub Check: tests (1, ubuntu-latest, py313)
  • GitHub Check: tests (1, windows-2022, py313)
  • GitHub Check: tests (2, ubuntu-latest, py311)
  • GitHub Check: tests (1, macos-latest, py313)
  • GitHub Check: tests (1, ubuntu-latest, py311)
  • GitHub Check: tests (1, ubuntu-latest, py312)
  • GitHub Check: apidocs
🔇 Additional comments (1)
src/snakemake/io/__init__.py (1)

469-477: LGTM! Appropriate revert of breaking change.

The change correctly reverts the overly strict behavior introduced in 9.11.9. Key strengths:

  • Warning instead of error allows workflows to continue when empty files are unused
  • Rule context is safely extracted with the if self.rule is not None guard
  • Message is clear and actionable, suggesting the proper alternative (empty list)
  • Execution continues to check for other file path issues, which is appropriate

This aligns well with the PR objective of treating empty-but-unused inputs as a warning until a proper deprecation cycle or major version bump.


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
Copy link
Copy Markdown
Contributor

fair enough, makes sense.

@johanneskoester johanneskoester merged commit 83c05cc into main Oct 30, 2025
60 checks passed
@johanneskoester johanneskoester deleted the revert-emptyerror-breaking-change branch October 30, 2025 15:41
johanneskoester pushed a commit that referenced this pull request Nov 4, 2025
🤖 I have created a release *beep* *boop*
---


##
[9.13.5](v9.13.4...v9.13.5)
(2025-11-04)


### Bug Fixes

* cache wrapper files and wait for them in case of shared filesystem for
sources ([#3809](#3809))
([498fff7](498fff7))
* correctly handle meta-wrapper tag replacement depending on the used
snakemake-wrapper release
([#3826](#3826))
([8d46a4a](8d46a4a))
* ensure that flags are properly considered for input files before
applying path modifiers (i.e. default storage providers)
([#3813](#3813))
([d3bfe32](d3bfe32))
* ensure that tokens are not leaked when paths or uris of source files
are logged ([#3821](#3821))
([a217e50](a217e50))
* print secs as numeric in jsonl benchmark format
([#3814](#3814))
([395a5e6](395a5e6))
* revert breaking change in 9.11.9 disallowing empty input files even
when unused
([#3810](#3810))
([83c05cc](83c05cc))
* shorten report ids (thus dir names) in order to avoid issues with path
length on windows
([#3822](#3822))
([b24d971](b24d971))

---
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
…ven when unused (snakemake#3810)

Reverts a breaking change introduced in version 9.11.9 where all input
files were required to be non-empty, even if they were not actually used
by a rule.

snakemake#3801 suggested that it would be acceptable to raise an error for this
in the future, as long as there is a deprecation warning period or it
comes with a major version bump, and a warning sounds good for now.

Thanks @corneliusroemer for pointing this out, and apologies for my
earlier overzealous modification.


### 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.
* [ ] 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**
* Improved handling of empty file paths by providing informative
guidance instead of raising an error. The system now logs a warning
message directing users to use an empty list ([]) to indicate no files,
making workflows more forgiving and easier to debug.

<!-- 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.5](snakemake/snakemake@v9.13.4...v9.13.5)
(2025-11-04)


### Bug Fixes

* cache wrapper files and wait for them in case of shared filesystem for
sources ([snakemake#3809](snakemake#3809))
([498fff7](snakemake@498fff7))
* correctly handle meta-wrapper tag replacement depending on the used
snakemake-wrapper release
([snakemake#3826](snakemake#3826))
([8d46a4a](snakemake@8d46a4a))
* ensure that flags are properly considered for input files before
applying path modifiers (i.e. default storage providers)
([snakemake#3813](snakemake#3813))
([d3bfe32](snakemake@d3bfe32))
* ensure that tokens are not leaked when paths or uris of source files
are logged ([snakemake#3821](snakemake#3821))
([a217e50](snakemake@a217e50))
* print secs as numeric in jsonl benchmark format
([snakemake#3814](snakemake#3814))
([395a5e6](snakemake@395a5e6))
* revert breaking change in 9.11.9 disallowing empty input files even
when unused
([snakemake#3810](snakemake#3810))
([83c05cc](snakemake@83c05cc))
* shorten report ids (thus dir names) in order to avoid issues with path
length on windows
([snakemake#3822](snakemake#3822))
([b24d971](snakemake@b24d971))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
@Hocnonsense Hocnonsense self-assigned this Mar 18, 2026
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.

2 participants