fix: corner case bug causing key error when module statement is used without config directive; better error message when empty string is defined as input or output file by Hocnonsense · Pull Request #3792 · snakemake/snakemake

Hocnonsense · 2025-10-13T05:43:12Z

will close #3774 and close #3754

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

example for returning an empty list to indicate no input can be found in https://snakemake.readthedocs.io/en/stable/snakefiles/rules.html#the-exists-function

Summary by CodeRabbit

Bug Fixes
- Improved error message for empty file paths, guiding users to use an empty list to indicate “no file.”
- Module configuration now defaults to an empty config when none is provided, improving predictability.
Tests
- Updated module tests to cover multiple configfile declarations and missing configs.
- Test environments pinned to Python 3.13 to ensure consistent test runs.

…3754

coderabbitai · 2025-10-13T05:43:39Z

📝 Walkthrough

Walkthrough

Updated IO error text for empty file paths; ensured module initialization always provides a config dict and always spawns a new checkpoints namespace; adjusted module parent-modifier assignment and skip_configfile logic; updated a module test Snakefile to include two configfile declarations; pinned Python in three notebook test envs to 3.13.

Changes

Cohort / File(s)	Summary
IO empty-path messaging `src/snakemake/io/__init__.py`	`_IOFile.check` now raises an error with updated text when `self._file == ""`, instructing to use an empty list (`[]`) instead of an empty string (`''`) to indicate “no file”.
Module initialization and config handling `src/snakemake/modules.py`	`WorkflowModifier.__init__` now always assigns `globals["config"] = config if config is not None else {}`; collapses parent modifier assignment into `self.parent_modifier = parent_modifier = workflow.modifier`; unconditionally calls `globals["checkpoints"] = globals["checkpoints"].spawn_new_namespace()` and ties `skip_configfile` to presence of the parent modifier.
Tests: module configfile declarations `tests/test_modules_all/module-test/Snakefile`	Replaced a single `configfile` entry with two `configfile:` declarations (`"config/config.yaml"`, `"config.yaml"`) and added a leading comment line.
Tests: notebook environment Python pinning `tests/test_jupyter_notebook/env.yaml`, `tests/test_jupyter_notebook_draft/env.yaml`, `tests/test_jupyter_notebook_nbconvert/env.yaml`	Pinned Python version from `>=3.5` to exactly `3.13` in three test environment YAML files; no other dependency changes.

Sequence Diagram(s)

sequenceDiagram
  participant WF as Workflow
  participant WM as WorkflowModifier
  participant Mod as Module Snakefile

  Note over WF,WM: Module initialization flow (high level)
  WF->>WM: instantiate WorkflowModifier(is_module, config, workflow)
  alt module initialization
    WM->>WM: globals["config"] = config or {}
    WM->>WM: globals["checkpoints"] = globals["checkpoints"].spawn_new_namespace()
    WF->>WM: workflow.modifier
    WM->>WM: parent_modifier = workflow.modifier
    WM->>WM: skip_configfile = (parent_modifier != None)
  else non-module path
    WF->>WM: workflow.modifier
    WM->>WM: parent_modifier = workflow.modifier
    WM->>WM: skip_configfile = (parent_modifier != None)
  end
  opt not skip_configfile
    WM->>Mod: evaluate `configfile:` statements inside module
  end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

fix: checkpoints inside modules are overwritten #3359 — touches checkpoint namespace handling and spawn_new_namespace usage, closely related to module checkpoint changes.
fix: Convert Path to IOFile #3405 — modifies IOFile behavior and Path→IOFile conversion, related to the IO empty-path message change.

Suggested reviewers

johanneskoester

Pre-merge checks and finishing touches

❌ Failed checks (3 warnings)

Check name	Status	Explanation	Resolution
Linked Issues Check	⚠️ Warning	The change to `modules.py` now guarantees a default empty config dictionary when no `config:` argument is provided, fully satisfying issue #3754, but for issue #3774 the patch only updates the error message advising use of an empty list without implementing or testing the actual sentinel handling of `[]` for absent inputs. As a result, only one of the two linked issue objectives is completely addressed.	Implement the logic to accept and correctly handle an empty list returned by input functions as the “no input” sentinel and add corresponding tests to verify this behavior so that issue #3774 is fully resolved alongside #3754.
Out of Scope Changes Check	⚠️ Warning	The modifications to pin the Python version in multiple `env.yaml` files under `tests/test_jupyter_notebook*` do not relate to improving empty-path handling or module config defaults and therefore fall outside the scope of issues #3774 and #3754. These environment changes introduce unrelated adjustments that should not be part of this PR focused on sentinel inputs and module configuration fixes.	Remove or isolate the Python version pinning changes in the test environments from this PR and revert the unrelated `env.yaml` updates so that the pull request remains focused solely on the code changes required by the linked issues.
Description Check	⚠️ Warning	The PR description does not follow the repository template’s structure and omits a clear summary of the changes made; it merely states the issues closed and includes a single example without detailing how each was addressed. The test-case checklist remains unchecked and lacks justification, and the placeholder for the description section has not been filled out. Overall, the description is incomplete and does not provide sufficient context for reviewers.	Add a clear description section at the top that outlines the specific code changes for empty-list sentinel support and default module config behavior, complete or justify the test-case checklist item, and ensure the description fully adheres to the provided template.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.
Title Check	✅ Passed	The pull request title accurately describes the two main changes in the changeset. It correctly identifies the first fix: addressing a KeyError when module statements lack a config directive (corresponding to the changes in src/snakemake/modules.py). It also identifies the second fix: improving the error message when empty strings are used as file paths (corresponding to changes in src/snakemake/io/init.py). The title is specific and concrete rather than vague, directly referencing the actual issues being resolved. While the title is somewhat lengthy at 28 words, it covers all substantive changes and would allow a developer scanning the PR history to understand the purpose of the changeset.

✨ 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-small-issues

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

🤖 I have created a release *beep* *boop* --- ## [9.13.3](v9.13.2...v9.13.3) (2025-10-17) ### Bug Fixes * corner case bug causing key error when module statement is used without config directive; better error message when empty string is defined as input or output file ([#3792](#3792)) ([640c549](640c549)) * make `--containerize` consider conda env specs from all rules, not just from dry-run DAG jobs ([#3783](#3783)) ([d092d8c](d092d8c)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

…without config directive; better error message when empty string is defined as input or output file (snakemake#3792) will close snakemake#3774 and close snakemake#3754 ### QC * [ ] 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). example for returning an empty list to indicate no input can be found in https://snakemake.readthedocs.io/en/stable/snakefiles/rules.html#the-exists-function  ## Summary by CodeRabbit * **Bug Fixes** * Improved error message for empty file paths, guiding users to use an empty list to indicate “no file.” * Module configuration now defaults to an empty config when none is provided, improving predictability. * **Tests** * Updated module tests to cover multiple configfile declarations and missing configs. * Test environments pinned to Python 3.13 to ensure consistent test runs.

🤖 I have created a release *beep* *boop* --- ## [9.13.3](snakemake/snakemake@v9.13.2...v9.13.3) (2025-10-17) ### Bug Fixes * corner case bug causing key error when module statement is used without config directive; better error message when empty string is defined as input or output file ([snakemake#3792](snakemake#3792)) ([640c549](snakemake@640c549)) * make `--containerize` consider conda env specs from all rules, not just from dry-run DAG jobs ([snakemake#3783](snakemake#3783)) ([d092d8c](snakemake@d092d8c)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

Hocnonsense added 2 commits October 13, 2025 12:06

fix: robust disable configfile even no config: specified in module #…

0f99911

…3754

docs: clearly desc for an empty string #3774

eee57b6

Hocnonsense requested a review from johanneskoester as a code owner October 13, 2025 05:43

johanneskoester reviewed Oct 14, 2025

View reviewed changes

Comment thread src/snakemake/modules.py Outdated

Hocnonsense added 3 commits October 14, 2025 23:53

Merge branch 'main' into fix-small-issues

38d6522

fix: don't skip if no config given

506502e

test: not py3.14 yet

ef72a55

Hocnonsense requested a review from johanneskoester October 14, 2025 16:14

johanneskoester approved these changes Oct 17, 2025

View reviewed changes

johanneskoester changed the title ~~fix: small issues~~ fix: corner case bug causing key error when module statement is used without config directive; better error message when empty string is defined as input or output file Oct 17, 2025

johanneskoester merged commit 640c549 into main Oct 17, 2025
61 checks passed

johanneskoester deleted the fix-small-issues branch October 17, 2025 17:14

snakemake-bot mentioned this pull request Oct 17, 2025

chore(main): release 9.13.3 #3794

Merged

Hocnonsense self-assigned this Mar 22, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: corner case bug causing key error when module statement is used without config directive; better error message when empty string is defined as input or output file#3792

fix: corner case bug causing key error when module statement is used without config directive; better error message when empty string is defined as input or output file#3792
johanneskoester merged 5 commits intomainfrom
fix-small-issues

Hocnonsense commented Oct 13, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 13, 2025 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Hocnonsense commented Oct 13, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

QC

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Hocnonsense commented Oct 13, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 13, 2025 •

edited

Loading