fix: display group jobs on dryrun#3435
Conversation
📝 WalkthroughWalkthroughThis PR updates the execution context check within the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Rule
participant Workflow
participant Settings as GroupSettings
Caller->>Rule: Access group property
Rule->>Workflow: Evaluate non_local_exec_or_dryrun
Workflow-->>Rule: Boolean result
alt Execution context is local and not dry run
Rule-->>Caller: Return None
else Execution context is non-local or dry run
Rule->>Workflow: Retrieve overwrite_group setting
Workflow->>Settings: Check for overwrite group
Settings-->>Workflow: Return overwrite value (if exists)
alt Overwrite group exists
Workflow-->>Rule: Return overwrite group
else No overwrite group found
Rule-->>Caller: Return internal _group
end
end
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Suggested reviewers
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
It appears that if we
dryrun fails with the following message: (adding the In all other cases it works, and is an improvement on the previous dryrun output. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/snakemake/workflow.py (1)
423-426: LGTM! Clean implementation that serves the intended purpose.The new property correctly combines the logic for non-local execution and dry run mode, which aligns with the PR objective to fix group job display during dry runs. The implementation is consistent with other similar properties in the class.
Optional improvement: Consider adding a docstring for clarity.
While not strictly necessary given the descriptive name, adding a brief docstring could help future maintainers understand the specific use case:
@property def non_local_exec_or_dryrun(self): + """Return True if execution is non-local or in dry run mode.""" return not self.local_exec or self.dryrun🧰 Tools
🪛 Pylint (3.3.7)
[convention] 424-424: Missing function or method docstring
(C0116)
src/snakemake/rules.py (1)
160-168: Refactor: Remove unnecessary else clause.The
elseclause is unnecessary since theifblock contains areturnstatement. The code can be simplified by removing theelseand de-indenting the subsequent code.@property def group(self): if not self.workflow.non_local_exec_or_dryrun: return None - else: - overwrite_group = self.workflow.group_settings.overwrite_groups.get( - self.name - ) - if overwrite_group is not None: - return overwrite_group - return self._group + overwrite_group = self.workflow.group_settings.overwrite_groups.get( + self.name + ) + if overwrite_group is not None: + return overwrite_group + return self._group🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 160-168: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/snakemake/rules.py(1 hunks)src/snakemake/workflow.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.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 `s...
**/*.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 theselfargument of methods.
Do not suggest type annotation of theclsargument of classmethods.
Do not suggest return type annotation if a function or method does not contain areturnstatement.
src/snakemake/rules.pysrc/snakemake/workflow.py
🧬 Code Graph Analysis (1)
src/snakemake/rules.py (1)
src/snakemake/workflow.py (1)
non_local_exec_or_dryrun(424-425)
🪛 Pylint (3.3.7)
src/snakemake/rules.py
[refactor] 160-168: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
src/snakemake/workflow.py
[convention] 424-424: Missing function or method docstring
(C0116)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: tests (9, macos-latest, py312)
- GitHub Check: tests (8, macos-latest, py312)
- GitHub Check: tests (6, macos-latest, py312)
- GitHub Check: tests (6, ubuntu-latest, py312)
- GitHub Check: tests (6, ubuntu-latest, py311)
- GitHub Check: tests (5, ubuntu-latest, py312)
- GitHub Check: tests (5, ubuntu-latest, py311)
- GitHub Check: tests (2, ubuntu-latest, py312)
- GitHub Check: tests (2, ubuntu-latest, py311)
🔇 Additional comments (1)
src/snakemake/rules.py (1)
160-160: LGTM! Logic correctly enables group jobs during dry runs.The change from
not self.workflow.local_exectonot self.workflow.non_local_exec_or_dryruncorrectly implements the intended functionality. Groups are now enabled during dry runs (dryrun=True) while preserving the original behavior for non-local execution scenarios.🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 160-168: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
I misunderstood the error message. Supplying the |
🤖 I have created a release *beep* *boop* --- ## [9.6.0](v9.5.1...v9.6.0) (2025-06-16) ### Features * Prefer papermill to nbconvert ([#2857](#2857)) ([4263b03](4263b03)) ### Bug Fixes * DeprecationWarning when using snakemake.utils.validate ([#3420](#3420)) ([cf72427](cf72427)) * display group jobs on dryrun ([#3435](#3435)) ([3bebef4](3bebef4)) * expandvars for special profile keys ([#3597](#3597)) ([4020188](4020188)) * fix bug causing --precommand to not being executed before each remote job ([#3625](#3625)) ([e59d125](e59d125)) * improved toggle switch behavior in reports ([#3623](#3623)) ([0c4bd23](0c4bd23)) * pass envvars defined via the envvars directive to remote jobs ([#3626](#3626)) ([d4890b4](d4890b4)) * remove wms arg, update logging cli docs ([#3622](#3622)) ([3a9a5ac](3a9a5ac)) * typo in CondaEnvDirSpec.__eq__ (issue [#3192](#3192)) ([#3613](#3613)) ([f4c107f](f4c107f)) * Unclear handling of params overriding with inheritance ([#3624](#3624)) ([077ac4a](077ac4a)) ### Documentation * Added snakemake command to execute the rule plot_with_python ([#3608](#3608)) ([bd99c11](bd99c11)) * Updated Reporting Section of The Tutorial(Interaction, Visualization, and Reporting with Snakemake) ([#3606](#3606)) ([91e90ba](91e90ba)) * Updated Requirement Section of The Tutorial With Warning of Not Installing The Tools Manually ([#3607](#3607)) ([3bd114b](3bd114b)) * Updated Wrapper Version in Tutorial and Used Simple Rule For Consistency & Ease ([#3605](#3605)) ([b3bcc21](b3bcc21)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
### Description <!--Add a description of your PR here--> Closes snakemake#3422 ### 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 ## Summary by CodeRabbit - **New Features** - Introduced a new property to evaluate non-local execution and dry run scenarios. - **Bug Fixes** - Refined task grouping behavior to ensure accurate assignment based on the updated execution context. These updates aim to provide more consistent and predictable workflow behavior across varied execution modes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Tadas Bareikis <tadasbar@gmail.com>
🤖 I have created a release *beep* *boop* --- ## [9.6.0](snakemake/snakemake@v9.5.1...v9.6.0) (2025-06-16) ### Features * Prefer papermill to nbconvert ([snakemake#2857](snakemake#2857)) ([4263b03](snakemake@4263b03)) ### Bug Fixes * DeprecationWarning when using snakemake.utils.validate ([snakemake#3420](snakemake#3420)) ([cf72427](snakemake@cf72427)) * display group jobs on dryrun ([snakemake#3435](snakemake#3435)) ([3bebef4](snakemake@3bebef4)) * expandvars for special profile keys ([snakemake#3597](snakemake#3597)) ([4020188](snakemake@4020188)) * fix bug causing --precommand to not being executed before each remote job ([snakemake#3625](snakemake#3625)) ([e59d125](snakemake@e59d125)) * improved toggle switch behavior in reports ([snakemake#3623](snakemake#3623)) ([0c4bd23](snakemake@0c4bd23)) * pass envvars defined via the envvars directive to remote jobs ([snakemake#3626](snakemake#3626)) ([d4890b4](snakemake@d4890b4)) * remove wms arg, update logging cli docs ([snakemake#3622](snakemake#3622)) ([3a9a5ac](snakemake@3a9a5ac)) * typo in CondaEnvDirSpec.__eq__ (issue [snakemake#3192](snakemake#3192)) ([snakemake#3613](snakemake#3613)) ([f4c107f](snakemake@f4c107f)) * Unclear handling of params overriding with inheritance ([snakemake#3624](snakemake#3624)) ([077ac4a](snakemake@077ac4a)) ### Documentation * Added snakemake command to execute the rule plot_with_python ([snakemake#3608](snakemake#3608)) ([bd99c11](snakemake@bd99c11)) * Updated Reporting Section of The Tutorial(Interaction, Visualization, and Reporting with Snakemake) ([snakemake#3606](snakemake#3606)) ([91e90ba](snakemake@91e90ba)) * Updated Requirement Section of The Tutorial With Warning of Not Installing The Tools Manually ([snakemake#3607](snakemake#3607)) ([3bd114b](snakemake@3bd114b)) * Updated Wrapper Version in Tutorial and Used Simple Rule For Consistency & Ease ([snakemake#3605](snakemake#3605)) ([b3bcc21](snakemake@b3bcc21)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Description
Closes #3422
QC
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
Summary by CodeRabbit
New Features
Bug Fixes
These updates aim to provide more consistent and predictable workflow behavior across varied execution modes.