fix: implement Resources.setdefault, restoring backwards compatibility with old resources implementation by victorlin · Pull Request #3968 · snakemake/snakemake

victorlin · 2026-02-11T21:32:03Z

This provides backwards compatibility for attributes that previously used native dict before switching to the new Resources class in "fix: standardize handling of resources" (4cada18).

Follow-up to #3421

QC

The PR contains a test case for the changes or the changes are already covered by an existing test case.
- Question: would a standalone file test_resources_setdefault.py or folder test_resources_setdefault/Snakefile be more appropriate?
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

New Features
- Added dictionary‑style default initialization for resource management: requesting a key returns the existing resource or stores and returns a provided default.
- Preserves existing entries when defaults are requested.
- Automatically normalizes non-resource values when stored as defaults, reducing initialization boilerplate.

This provides backwards compatibility for attributes that previously used native dict before switching to the new Resources class in "fix: standardize handling of resources" (4cada18).

coderabbitai · 2026-02-11T21:32:24Z

📝 Walkthrough

Walkthrough

Added a new setdefault method to the Resources class that returns the existing Resource for a key or sets and returns a new Resource when the key is absent, using the class's existing __setitem__ behavior.

Changes

Resources class enhancement

Layer / File(s)	Summary
Resources setdefault method `src/snakemake/resources.py`	Added public `setdefault(self, ix: str, val: ValidResource \| Resource \| None)` method. Implements dict-like default initialization: if `ix` exists returns stored `Resource`, otherwise assigns via existing `__setitem__` (wrapping non-`Resource` values) and returns the stored `Resource`.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 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.
Description check	❓ Inconclusive	The PR description provides context but leaves critical QC checklist items incomplete with unchecked boxes and a question about test file structure, without confirmation of test or documentation changes.	Complete the QC checklist by confirming whether tests exist or were added, deciding on test file structure, and confirming documentation updates are not necessary. Check the corresponding boxes.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and specifically describes the main change: implementing Resources.setdefault method to restore backwards compatibility with the old resources implementation.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

coderabbitai

🧹 Nitpick comments (1)

src/snakemake/resources.py (1)
1128-1132: Implementation looks correct.

The method properly delegates to __setitem__ for value storage and correctly returns the stored Resource. This provides the expected dict-like setdefault behavior for backwards compatibility.

Consider adding a return type annotation for API clarity:
-    def setdefault(self, ix: str, val: ValidResource | Resource | None):
+    def setdefault(self, ix: str, val: ValidResource | Resource | None) -> Resource:
,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/snakemake/resources.py` around lines 1128 - 1132, The setdefault method
lacks a return type annotation; add an explicit return type (e.g., ->
ValidResource | Resource | None or the precise Resource type used in this
module) to the setdefault signature so callers and type checkers know what is
returned, keeping the implementation as-is and referencing the existing
setdefault method, __setitem__, and self._data for context.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/snakemake/resources.py`:
- Around line 1128-1132: The setdefault method lacks a return type annotation;
add an explicit return type (e.g., -> ValidResource | Resource | None or the
precise Resource type used in this module) to the setdefault signature so
callers and type checkers know what is returned, keeping the implementation
as-is and referencing the existing setdefault method, __setitem__, and
self._data for context.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ac45d0be-c7bb-4eb9-b082-47572046658d

📥 Commits

Reviewing files that changed from the base of the PR and between 143c253 and e68f413.

📒 Files selected for processing (1)

src/snakemake/resources.py

🤖 I have created a release *beep* *boop* --- ## [9.21.1](v9.21.0...v9.21.1) (2026-05-29) ### Bug Fixes * add default json function to benchmarks ([#4128](#4128)) ([41fab22](41fab22)) * do not rerun when checkpoint job missing but downstream file exists ([#4124](#4124)) ([a060b93](a060b93)) * ensure that error logs contain all available details ([#4183](#4183)) ([74a86e9](74a86e9)) * handle missing pss attribute in benchmark on Windows ([#4160](#4160)) ([da52080](da52080)) * implement Resources.setdefault ([#3968](#3968)) ([2413e99](2413e99)) * reporting remote nodes number ([#3978](#3978)) ([8c534f0](8c534f0)) * resolve pathvars before constructing storage queries ([#3969](#3969)) ([bd15237](bd15237)) * storage temp() file cleanup with RemoteProvider ([#4189](#4189)) ([898bad1](898bad1)) * tolerate FileNotFoundError in drop_iocache ([#4153](#4153)) ([#4191](#4191)) ([ce26b28](ce26b28)) ### Documentation * Added guide on debugging workflows ([#4029](#4029)) ([3d052ae](3d052ae)) * **cli:** Remove broken ref bold markup ([#4204](#4204)) ([1200ebf](1200ebf)) * remove duplicated resources attribute in rules.rst ([#4190](#4190)) ([6c8ecdd](6c8ecdd)) * **rules:** Update script type hint advice ([#4193](#4193)) ([6108712](6108712)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

fix: implement Resources.setdefault

143c253

This provides backwards compatibility for attributes that previously used native dict before switching to the new Resources class in "fix: standardize handling of resources" (4cada18).

victorlin requested a review from johanneskoester as a code owner February 11, 2026 21:32

victorlin mentioned this pull request Feb 11, 2026

Update workflows for Snakemake v9.16.3 nextstrain/public#37

Closed

3 tasks

fgvieira and others added 2 commits February 27, 2026 11:21

Merge branch 'main' into victorlin/update-resources

d68001c

Merge branch 'main' into victorlin/update-resources

e68f413

coderabbitai Bot reviewed Mar 30, 2026

View reviewed changes

johanneskoester approved these changes May 29, 2026

View reviewed changes

Merge branch 'main' into victorlin/update-resources

b17ab43

johanneskoester enabled auto-merge (squash) May 29, 2026 13:45

johanneskoester changed the title ~~fix: implement Resources.setdefault~~ fix: implement Resources.setdefault, restoring backwards compatibility with old resources implementation May 29, 2026

johanneskoester merged commit 2413e99 into snakemake:main May 29, 2026
61 checks passed

snakemake-bot mentioned this pull request May 29, 2026

chore(main): release 9.21.1 #4200

Merged

victorlin deleted the victorlin/update-resources branch May 29, 2026 19:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: implement Resources.setdefault, restoring backwards compatibility with old resources implementation#3968

fix: implement Resources.setdefault, restoring backwards compatibility with old resources implementation#3968
johanneskoester merged 4 commits into
snakemake:mainfrom
victorlin:victorlin/update-resources

victorlin commented Feb 11, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Feb 11, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

❌ Failed checks (1 warning, 1 inconclusive)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

victorlin commented Feb 11, 2026 • 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 Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

❌ Failed checks (1 warning, 1 inconclusive)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

victorlin commented Feb 11, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Feb 11, 2026 •

edited

Loading