feat: inherit parameters of use rule and extend/replace individual items them when using 'with' directive#3365
Conversation
📝 WalkthroughWalkthroughThis pull request refactors and extends various aspects of the Snakemake framework and its tests. In Changes
Sequence Diagram(s)sequenceDiagram
participant R as RuleInfo
participant M as Modifier
participant L as Logger
R->>R: Retrieve original positional & keyword params
M-->>R: Provide new positional & keyword params
alt Modifier provides positional parameters
R->>L: Log warning if original positional args exist
R->>R: Merge modifier's positional args with originals
end
R->>R: Merge keyword arguments (preserve existing unless overwritten)
sequenceDiagram
participant WF as Workflow
participant R as RuleInfo
WF->>R: Process rule via decorate()
alt ruleinfo.name is provided
WF->>WF: Update rule name and re-register rule
else
WF->>WF: Retain original rule name
end
Possibly related PRs
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/snakefiles/modularization.rst (1)
194-197: Clarify 'with' Directive Parameter Inheritance in DocumentationThe new note block clearly explains that:
- Non-specified keyword parameters from the original rule are inherited.
- Positional parameters provided in the modifying (with) clause will completely overwrite the original positional parameters.
To further improve clarity, consider adding an explicit example that demonstrates the difference between positional and keyword parameters. This can help users quickly understand how parameters will be retained or overwritten.
For instance, you might add something like:
.. note:: - Modification of `params` allows the replacement of single keyword arguments. Keyword `params` of the original rule that are not defined after `with` are inherited. Positional `params` arguments of the original rule are overwritten, if positional `params` arguments are given after `with`. - All other properties are overwritten with the values specified after `with`. + Modification of `params` allows the replacement of single keyword arguments. Keyword `params` of the original rule that are not defined after `with` are inherited. Positional `params` arguments of the original rule are overwritten, if positional `params` arguments are given after `with`. + All other properties are overwritten with the values specified after `with`. + + **Example:** + Original rule: + params: ("default_pos", key1="default_kw") + Modified with: + params: (key1="new_value") + In this case, the keyword argument `key1` is updated to `"new_value"`, while the positional parameter `"default_pos"` is retained.This additional example could help clarify the behavior for end users.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
tests/test_issue3338/expected-results/test.outis excluded by!**/*.outtests/test_issue3338/expected-results/test2.outis excluded by!**/*.outtests/test_issue3338/expected-results/test.outis excluded by!**/*.outtests/test_issue3338/expected-results/test2.outis excluded by!**/*.out
📒 Files selected for processing (7)
snakemake/ruleinfo.py(1 hunks)tests/test_issue3338/Snakefile(1 hunks)tests/test_issue3338/Snakefile(2 hunks)snakemake/ruleinfo.py(2 hunks)snakemake/workflow.py(1 hunks)docs/snakefiles/modularization.rst(1 hunks)tests/test_issue3338/Snakefile(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- snakemake/ruleinfo.py
- snakemake/ruleinfo.py
- tests/test_issue3338/Snakefile
- tests/test_issue3338/Snakefile
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.py`: Do not try to improve formatting. Do not suggest ...
**/*.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.
snakemake/workflow.py
🔇 Additional comments (5)
snakemake/workflow.py (1)
635-636: Added essential rule name assignment.This change ensures that the rule name is properly assigned to the
ruleinfoobject before applying any modifiers. This is critical for the PR objective of retaining parameters when using the 'with' directive, as the rule name is needed when handling parameter inheritance in the modifier application process.tests/test_issue3338/Snakefile (4)
4-6: Added new output files to the all rule.These additions correctly expand the all rule to include the new test output files that will be created by the test rules for parameter inheritance.
8-8: Clear comment for test purpose.Good documentation that explains this test's purpose - verifying parameter inheritance works for workflows imported as modules.
19-28: Added new test rule for parameter inheritance.This rule
aappropriately defines multiple parameters (a=3,b=6) to serve as the base rule for testing parameter inheritance. The shell command uses both parameters to verify they're correctly passed to the execution context.
29-33: Added rule inheritance test case.This is an excellent test case for the PR's objective. Rule
bextends ruleabut only overrides parametera(setting it to 9), while implicitly expecting to inherit parameterb=6from rulea. This directly tests the fix to ensure parameters are retained when using the 'with' directive.
|
Make sure to run black on the codebase in order to adjust the formatting. |
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
docs/snakefiles/reporting.rst (1)
7-330: 💡 Verification agent🧩 Analysis chain
Documentation changes unrelated to PR objective
While the documentation changes are excellent, they focus on reporting features rather than the PR objective of retaining parameters when using the 'with' directive. This suggests either missing documentation updates or a mismatch between the PR description and the actual changes.
🌐 Web query:
What is the 'with' directive in Snakemake?💡 Result:
The
withdirective in Snakemake is used to modify imported rules when leveraging theuse rulestatement for modular workflows. It allows customization of specific rule properties (e.g.,input,output,params) from an external module without altering the original rule definition.Key Features:
Rule Customization
- Overwrite directives like
input,output, orparamsof imported rules.
Example:use rule samtools_sort from external_module with: input: "modified_input.txt"Integration with Modules
Used in combination withmoduledeclarations to import and adapt rules from external workflows.
Example:module external_workflow: snakefile: "path/to/Snakefile" use rule * from external_workflow as prefix_* with: params: custom_param=42Selective Overrides
Enables partial modifications while retaining other properties of the original rule.
Example:use rule bcftools_call from genomics_module with: output: "custom_output.vcf"Limitations:
- Cannot modify the execution method (
shell,script,run, etc.) of the original rule.- Requires explicit naming via
aswhen importing rules to avoid conflicts.Use Case Example:
module bwa_mapping: meta_wrapper: "0.72.0/meta/bio/bwa_mapping" use rule bwa_mem from bwa_mapping with: input: lambda wildcards: config["samples"][wildcards.sample]Here, the
withdirective redefines theinputof thebwa_memrule from thebwa_mappingmodule to align with the current workflow's configuration[10][12].This feature enhances code reuse and adaptability in complex workflows by allowing rule customization without duplicating code.
Citations:
- 1: https://snakemake.readthedocs.io/en/stable/tutorial/basics.html
- 2: https://forum.reana.io/t/snakemake-workflows-in-reana/60
- 3: https://snakemake.readthedocs.io/en/stable/snakefiles/deployment.html
- 4: https://submit.mit.edu/submit-users-guide/tutorials/tutorial_7.html
- 5: https://snakemake.readthedocs.io/en/v5.32.0/tutorial/advanced.html
- 6: https://snakemake.readthedocs.io/en/v6.1.1/snakefiles/modularization.html
- 7: https://snakemake.readthedocs.io/en/stable/snakefiles/rules.html
- 8: https://snakemake.readthedocs.io/en/stable/snakefiles/reporting.html
- 9: https://snakemake.readthedocs.io/en/stable/tutorial/advanced.html
- 10: https://snakemake.readthedocs.io/en/stable/snakefiles/modularization.html
- 11: https://snakemake.readthedocs.io/en/stable/tutorial/additional_features.html
- 12: https://snakemake.readthedocs.io/en/v6.12.1/snakefiles/rules.html
- 13: https://snakemake.readthedocs.io/en/master/snakefiles/rules.html
Documentation Misalignment: Missing 'with' Directive Parameter Retention
The documentation update in
docs/snakefiles/reporting.rstprovides comprehensive details on report generation and associated features. However, it currently does not address the PR objective of retaining parameters when using thewithdirective—which is used to modify imported rule properties without altering the original rule. This creates a gap between the intended PR feature and the updated documentation.
- Action Required:
- Update or expand the documentation to include information on retaining parameters when using the
withdirective (i.e., how parameters, such asinput,output, orparams, are preserved or modified).- Alternatively, if the PR solely targets reporting changes, clarify the objective to avoid confusion.
🧹 Nitpick comments (2)
snakemake/workflow.py (1)
1814-1815: Consider using a consistent validation approach for global container images.The check for global container image is implemented slightly differently than the other validation checks. Consider refactoring to use the
check_may_use_software_deploymenthelper function here as well for consistency.Also, the static analyzer flagged comparison
!= Falsewhich could be simplified.-if not ruleinfo.template_engine and ruleinfo.container_img != False: +if not ruleinfo.template_engine and ruleinfo.container_img:🧰 Tools
🪛 Ruff (0.8.2)
1814-1814: Avoid inequality comparisons to
False; useif ruleinfo.container_img:for truth checksReplace with
ruleinfo.container_img(E712)
docs/snakefiles/rules.rst (1)
1592-1611: New Xonsh Documentation Section Added
The additional Xonsh section clearly demonstrates how users can integrate Xonsh scripts in their Snakemake rules. The code example—with its clear structure and reference link to Xonsh—provides a solid illustration of usage.Suggestion: Consider adding a brief remark on when a user might prefer using a Xonsh script over a regular Python or shell script. This can further empower users in choosing the best tool for their workflow needs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
CHANGELOG.mdis excluded by!CHANGELOG.mdpyproject.tomlis excluded by!pyproject.tomltests/test_script_xsh/expected-results/test.outis excluded by!**/*.out
📒 Files selected for processing (19)
snakemake/workflow.py(1 hunks)docs/snakefiles/deployment.rst(2 hunks)docs/snakefiles/reporting.rst(5 hunks)docs/snakefiles/rules.rst(1 hunks)setup.py(1 hunks)snakemake/cli.py(1 hunks)snakemake/script/__init__.py(3 hunks)snakemake/workflow.py(1 hunks)test-environment.yml(2 hunks)tests/test_conda_python_3_7_script/Snakefile(1 hunks)tests/test_conda_python_3_7_script/test_script_python_3_7.py(1 hunks)tests/test_conda_run/Snakefile(1 hunks)tests/test_conda_run/expected-results/test.txt(1 hunks)tests/test_conda_run/test_python_env.yaml(1 hunks)tests/test_conda_run/test_script_run.py(1 hunks)tests/test_script_xsh/Snakefile(1 hunks)tests/test_script_xsh/envs/xonsh.yaml(1 hunks)tests/test_script_xsh/scripts/test.xsh(1 hunks)tests/tests_using_conda.py(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- tests/test_conda_run/expected-results/test.txt
- tests/test_conda_python_3_7_script/test_script_python_3_7.py
- tests/test_script_xsh/scripts/test.xsh
- tests/test_conda_run/test_script_run.py
- tests/test_conda_run/test_python_env.yaml
- tests/test_script_xsh/envs/xonsh.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- snakemake/workflow.py
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.py`: Do not try to improve formatting. Do not suggest ...
**/*.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.
setup.pysnakemake/cli.pytests/tests_using_conda.pysnakemake/script/__init__.pysnakemake/workflow.py
🪛 Ruff (0.8.2)
snakemake/workflow.py
1814-1814: Avoid inequality comparisons to False; use if ruleinfo.container_img: for truth checks
Replace with ruleinfo.container_img
(E712)
🔇 Additional comments (28)
snakemake/workflow.py (4)
1781-1788: LGTM: Good introduction of helper function for validation.This new helper function centralizes the logic for checking if software deployment directives are compatible with template engines, which improves code maintainability and avoids duplication.
1796-1796: Good reuse of the helper function for envmodules validation.Calling the shared validation function provides consistency in how rule validations are performed.
1802-1802: Good reuse of the helper function for conda validation.Calling the shared validation function provides consistency in how rule validations are performed.
1810-1810: Good reuse of the helper function for container/singularity validation.Calling the shared validation function provides consistency in how rule validations are performed.
setup.py (1)
33-33: Addition of unit test templates to package data looks good.This change ensures that the template files in the unit_tests/templates directory are included when the package is built and installed, which is necessary for the tests related to issue #3338 to work correctly.
tests/test_conda_python_3_7_script/Snakefile (1)
7-7: Script name update is appropriate.Updating the script file name to explicitly mention Python 3.7 improves clarity about the test's purpose and aligns with the conda environment defined in test_python_env.yaml.
tests/test_script_xsh/Snakefile (2)
1-4: Well-structured 'all' rule for the test workflow.The 'all' rule correctly defines the expected final output of the workflow, following Snakemake best practices.
6-12: Xonsh script test rule looks good.This rule properly sets up a test for Xonsh script execution within Snakemake, using the appropriate conda environment and script directive. This will help verify that parameters are correctly retained when using the 'with' directive with Xonsh scripts.
tests/test_conda_run/Snakefile (2)
1-8: Good use of parameters for script path.The rule correctly uses the
paramsdirective to store the script path, andworkflow.source_path()ensures the script is found relative to the Snakefile, which is a good practice.
9-13: Helpful comments explaining test pattern.These comments clearly explain that this pattern (calling a script from shell) is not recommended for real workflows, which helps prevent users from adopting this anti-pattern in their own code.
tests/tests_using_conda.py (2)
309-316: Good addition of xonsh script testing capability.The new
test_script_xshfunction correctly uses the@skip_on_windowsdecorator since xonsh scripts are primarily for Unix-like environments, and properly configures the conda deployment method.
318-321: LGTM - Appropriate test for conda_run functionality.This test follows the established pattern in the file and will help verify the conda run functionality.
test-environment.yml (2)
7-7: Good addition of setuptools dependency.Adding setuptools to the main dependencies is appropriate since it's a common requirement for many Python packages.
61-62: Improved dependency management by moving packages from pip to conda.Moving
cwltoolandcwl-utilsfrom pip dependencies to main conda dependencies is a better practice when these packages are available in conda channels, as it allows for better dependency resolution.snakemake/cli.py (1)
957-965: Enhanced help text with clearer guidance on report generation.The improved help text for the
--reportargument provides more detailed and useful information about:
- The self-contained nature of the reports
- When to use
.htmlvs.zipformat based on dataset size- How the folder structure in the zip archive works
This will help users make better decisions about report generation format.
docs/snakefiles/deployment.rst (1)
288-294: Clarification on Conda Integration with therunDirective
The updated note effectively clarifies that although conda environments can be used with several directives (i.e.shell,script,notebook,wrapper, andrun), therundirective is a special case. Emphasizing that therunblock executes in the same process as Snakemake—and that the conda directive in such cases only affectsshellfunction calls within the run block—is crucial for user understanding. The reminder regarding notebook usage and the prerequisite for havingjupyteravailable is also highly beneficial.Overall, these changes enhance clarity regarding the subtleties of environment activation in different directive contexts.
snakemake/script/__init__.py (5)
1600-1603: Great addition of XonshScript support!This implementation correctly adds support for Xonsh scripts by inheriting from PythonScript and overriding the execute_script method to use the
xonshcommand. Xonsh is a Python-based shell language that combines Python's power with shell scripting, making it a good fit for Snakemake workflows.
1667-1668: File extension detection correctly implementedThe addition of the
.xshextension detection follows the existing pattern in the get_language function and properly identifies Xonsh script files.
1732-1732: Proper mapping of language to executor classThe Xonsh script executor is correctly mapped in the exec_class dictionary, maintaining consistency with how other script types are handled.
1736-1737: Updated error message to include XonshThe error message has been appropriately updated to include Xonsh in the list of supported script types, providing clear guidance to users.
1600-1737:Details
❓ Verification inconclusive
IMPORTANT: No code changes related to PR objective
The PR objective mentions ensuring parameters are retained when using the 'with' directive in Snakemake, but none of the code changes in this file address that feature. This seems to be adding Xonsh script support instead.
🏁 Script executed:
#!/bin/bash # Search for files with changes related to the 'with' directive and parameters rg -l "with.*params" --type pyLength of output: 76
ACTION REQUIRED: Confirm Feature Separation and Parameter Retention Implementation
This file exclusively implements Xonsh script support via the new
XonshScriptclass and does not include any modifications for retaining parameters when using thewithdirective. The PR objective indicates that parameter retention is a key requirement, yet the changes addressing this feature appear to be located in other parts of the codebase. In fact, the search output shows matches in the following files:
snakemake/rules.pysnakemake/persistence.pyPlease verify that:
- The parameter retention logic for the
withdirective has been fully implemented in those files.- The separation between adding Xonsh script support and modifying parameter handling is intentional and consistent with the overall PR objectives.
If the intended functionality for retaining parameters is not adequately covered by the changes in
snakemake/rules.pyandsnakemake/persistence.py, additional modifications might be necessary.🧰 Tools
🪛 Ruff (0.8.2)
1675-1675: Local variable
eis assigned to but never usedRemove assignment to unused variable
e(F841)
1676-1679: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
1725-1733: Use
dict.get()without default valueRemove default value
(SIM910)
docs/snakefiles/reporting.rst (7)
7-10: Improved introduction clarityThe introduction has been enhanced to better explain what Snakemake reports include by default (runtime statistics, provenance information, workflow topology) and clarifies the different report types available (HTML file vs ZIP archive).
13-17: Good section organizationAdding a dedicated section for "Including results in a report" with proper restructured text formatting improves documentation organization and discoverability.
257-266: Better section hierarchy with proper anchorsAdding the _snakefiles-rendering_reports anchor and the _snakefiles-self_contained_html_file anchor improves navigation within the documentation, making it easier to link to specific sections.
267-285: Clear explanation of self-contained HTML reportsThis section provides excellent guidance on generating basic HTML reports, including how to customize the report filename. The note about only using this format for smaller reports is particularly helpful for users.
286-303: Good explanation of ZIP archive reportsThis new section properly documents the ZIP archive report option, explaining when to use it (complex reports with many samples) and how to navigate the resulting archive. This information is valuable for users dealing with large analyses.
304-317: Useful information on partial reportsThe addition of information about generating reports for specific targets provides valuable workflow flexibility. The explanation of use cases (exploring intermediate outputs, alternative target rules) helps users understand when this feature would be useful.
318-330: Complete documentation of custom stylingThis section on custom layouts with stylesheets is well documented, with clear examples and even downloadable resources. Including references to the full example source code is particularly helpful.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
snakemake/ioutils/input.py (1)
12-31:⚠️ Potential issueFix several issues in the extract_checksum function
There are a few issues with this implementation:
- Missing import for
WorkflowError- Lambda function should be defined as a regular function
- Exception should be raised with original cause
Here's how to fix these issues:
+from snakemake.exceptions import WorkflowError def extract_checksum(infile, **kwargs): try: import pandas as pd - fix_file_name = lambda x: x.removeprefix("./") + def fix_file_name(x): + return x.removeprefix("./") return ( pd.read_csv( infile, sep=" ", header=None, engine="python", converters={1: fix_file_name}, ) .set_index(1) .loc[fix_file_name(kwargs.get("file"))] .item() ) except ImportError as err: - raise WorkflowError("Pandas is required to extract checksum from file.") + raise WorkflowError("Pandas is required to extract checksum from file.") from err🧰 Tools
🪛 Ruff (0.8.2)
16-16: Do not assign a
lambdaexpression, use adefRewrite
fix_file_nameas adef(E731)
30-30: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
30-30: Undefined name
WorkflowError(F821)
🧹 Nitpick comments (4)
snakemake/workflow.py (1)
1814-1816: Use Python truth value testing instead of explicit comparison to False.Python style guides recommend using truth value testing instead of explicit equality comparison with boolean values.
- if not ruleinfo.template_engine and ruleinfo.container_img != False: + if not ruleinfo.template_engine and ruleinfo.container_img:🧰 Tools
🪛 Ruff (0.8.2)
1815-1815: Avoid inequality comparisons to
False; useif ruleinfo.container_img:for truth checksReplace with
ruleinfo.container_img(E712)
.github/workflows/main.yml (3)
51-54: Dash shell testing configuration is commented out but not removedThe dash shell configuration is commented out rather than properly removed. This could lead to confusion about whether dash testing is intended to be temporarily disabled or permanently removed.
Either completely remove the commented code or add a comment explaining why it's temporarily disabled and when it's expected to be re-enabled.
shell: - bash - #- dash # for py 3.12 also test dash as default shell (snakemake will still use bash for shell commands) exclude: [] - #- shell: dash - # py_ver: "3.11"🧰 Tools
🪛 actionlint (1.7.4)
52-52: "exclude" section should not be empty
(syntax-check)
🪛 YAMLlint (1.35.1)
[error] 53-53: trailing spaces
(trailing-spaces)
52-52: Empty exclude sectionThe
excludesection is now empty but retained in the configuration. This could be simplified.Consider removing the empty exclude section entirely since it no longer serves a purpose:
shell: - bash exclude: []🧰 Tools
🪛 actionlint (1.7.4)
52-52: "exclude" section should not be empty
(syntax-check)
218-218: Missing newline at end of fileThe file is missing a newline at the end, which is a common convention for text files and can cause issues with some tools.
Add a newline at the end of the file.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 218-218: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
snakemake/workflow.py(1 hunks)docs/snakefiles/rules.rst(1 hunks)snakemake/cli.py(1 hunks)snakemake/workflow.py(1 hunks).github/workflows/codespell.yml(1 hunks).github/workflows/docs.yml(2 hunks).github/workflows/main.yml(6 hunks)docs/snakefiles/rules.rst(2 hunks)snakemake/cli.py(1 hunks)snakemake/ioutils/__init__.py(2 hunks)snakemake/ioutils/input.py(1 hunks)snakemake/workflow.py(1 hunks)tests/test_ioutils/Snakefile(4 hunks)tests/test_ioutils/expected-results/c/1.txt(1 hunks)tests/test_ioutils/expected-results/results/switch~someswitch.column~sample.txt(1 hunks)tests/test_ioutils/samples.md5(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- tests/test_ioutils/expected-results/results/switch
someswitch.columnsample.txt - tests/test_ioutils/expected-results/c/1.txt
- tests/test_ioutils/samples.md5
🚧 Files skipped from review as they are similar to previous changes (5)
- snakemake/workflow.py
- snakemake/cli.py
- snakemake/cli.py
- docs/snakefiles/rules.rst
- docs/snakefiles/rules.rst
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.py`: Do not try to improve formatting. Do not suggest ...
**/*.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.
snakemake/workflow.pysnakemake/ioutils/input.pysnakemake/ioutils/__init__.py
🪛 Ruff (0.8.2)
snakemake/workflow.py
1815-1815: Avoid inequality comparisons to False; use if ruleinfo.container_img: for truth checks
Replace with ruleinfo.container_img
(E712)
snakemake/ioutils/input.py
16-16: Do not assign a lambda expression, use a def
Rewrite fix_file_name as a def
(E731)
30-30: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
30-30: Undefined name WorkflowError
(F821)
🪛 actionlint (1.7.4)
.github/workflows/main.yml
52-52: "exclude" section should not be empty
(syntax-check)
🪛 YAMLlint (1.35.1)
.github/workflows/main.yml
[error] 53-53: trailing spaces
(trailing-spaces)
[error] 218-218: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (14)
snakemake/workflow.py (4)
1781-1789: Good addition of a helper function for validation.This is a good refactoring that centralizes validation for software deployment directives, ensuring that they're only used with appropriate rule types. This improves code maintainability and provides clearer error messages.
1796-1796: LGTM: Proper validation for envmodules.Good use of the new helper function to validate envmodules usage.
1802-1802: LGTM: Proper validation for conda.Good use of the new helper function to validate conda usage.
1810-1810: LGTM: Proper validation for container/singularity.Good use of the new helper function to validate container/singularity usage.
.github/workflows/codespell.yml (1)
16-16: Efficient CI workflow optimizationAdding this conditional statement is a good practice that prevents redundant CI runs when PRs are merged to the main branch, improving overall CI efficiency.
.github/workflows/docs.yml (1)
16-16: Consistent CI workflow optimizationAdding these conditional statements to both documentation jobs follows the same pattern as in the codespell workflow, preventing redundant CI runs for merged PRs. This systematic approach to optimizing CI workflows is beneficial for the overall efficiency of the CI pipeline.
Also applies to: 41-41
snakemake/ioutils/__init__.py (1)
8-8: Proper exposure of new utility functionsThe newly added import and global registration of
parse_inputandextract_checksumfunctions follow the established pattern in the codebase. This makes the new functions available to Snakemake workflows, supporting the enhanced functionality for the 'with' directive.Also applies to: 25-26
snakemake/ioutils/input.py (1)
1-10: Clean implementation of the parse_input functionThe
parse_inputfunction is well-implemented and follows Snakemake's pattern for rule input functions. It provides the flexibility to either use a custom parser or simply retrieve the file contents..github/workflows/main.yml (1)
16-16: Conditional execution is consistently applied to all jobsThe PR adds a consistent condition to skip jobs when a PR is merged or when running on the main branch, which prevents redundant CI runs. The addition of the
testing-donejob serves as a checkpoint to confirm that all tests have passed successfully.Also applies to: 42-42, 157-157, 167-167, 211-218
tests/test_ioutils/Snakefile (5)
11-13: New checksum validation testThis assertion tests the
extract_checksumfunction with a specific expected value. This is a good addition that validates the checksum functionality.
22-30: Enhanced rule 'a' with checksum handlingThe rule has been updated to handle checksums by:
- Adding a new input for the checksum file
- Using
parse_inputwithextract_checksumto process the checksum for the specific sampleThis change aligns with the PR objective of enhancing parameter handling.
31-31: Updated shell command to use checksum parameterThe shell command now outputs the dynamically computed checksum instead of a static value, which properly tests the parameter passing functionality.
58-58: Added comma after input itemA comma was added after the input item declaration, which is a good practice for maintaining consistent syntax even with single items in a collection.
85-85: Consistent shell command formattingThe shell command in rule 'e' maintains the same functionality but appears to have been reformatted for consistent line ending.
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.github/workflows/main.yml (3)
49-54: Review the 'exclude' Configuration in the Test MatrixThe
exclude: []section is currently empty. This might be redundant if no exclusions are necessary. Consider either removing the key entirely or adding a comment to explain its purpose. Also, YAMLlint flagged trailing spaces (e.g., on line 53); please remove any extraneous whitespace in these commented lines.🧰 Tools
🪛 actionlint (1.7.4)
52-52: "exclude" section should not be empty
(syntax-check)
🪛 YAMLlint (1.35.1)
[error] 53-53: trailing spaces
(trailing-spaces)
51-51: Consider Cleaning Up Commented-Out CodeThere is a commented-out line for the 'dash' shell test. If this alternative is no longer needed, removing it might improve clarity. Otherwise, adding a brief comment on its current relevance could be helpful.
211-218: Review the Newly Added 'testing-done' Job and File FormattingThe new
testing-donejob correctly depends on both the 'testing' and 'testing-windows' jobs and echoes a success message. Additionally, YAMLlint flagged that there is no newline character at the end of the file (line 218). Please add a newline to comply with standard file formatting practices.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 218-218: no new line character at the end of file
(new-line-at-end-of-file)
🛑 Comments failed to post (1)
snakemake/ioutils/input.py (1)
12-30:
⚠️ Potential issueFix WorkflowError import and lambda function definition.
There are a few issues in the
extract_checksumfunction:
WorkflowErroris undefined - needs to be imported- The lambda function should be defined as a regular function
- Exception handling should use "raise ... from err" syntax to preserve the traceback
Apply this diff to fix the issues:
+from snakemake.exceptions import WorkflowError def extract_checksum(infile, **kwargs): try: import pandas as pd - fix_file_name = lambda x: x.removeprefix("./") + def fix_file_name(x): + return x.removeprefix("./") return ( pd.read_csv( infile, sep=" ", header=None, engine="python", converters={1: fix_file_name}, ) .set_index(1) .loc[fix_file_name(kwargs.get("file"))] .item() ) except ImportError as err: - raise WorkflowError("Pandas is required to extract checksum from file.") + raise WorkflowError("Pandas is required to extract checksum from file.") from err📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.from snakemake.exceptions import WorkflowError def extract_checksum(infile, **kwargs): try: import pandas as pd def fix_file_name(x): return x.removeprefix("./") return ( pd.read_csv( infile, sep=" ", header=None, engine="python", converters={1: fix_file_name}, ) .set_index(1) .loc[fix_file_name(kwargs.get("file"))] .item() ) except ImportError as err: raise WorkflowError("Pandas is required to extract checksum from file.") from err🧰 Tools
🪛 Ruff (0.8.2)
16-16: Do not assign a
lambdaexpression, use adefRewrite
fix_file_nameas adef(E731)
30-30: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
30-30: Undefined name
WorkflowError(F821)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
tests/test_validate/samples.schema.yaml (1)
9-9: Consider preserving detailed property description.The description for the
conditionproperty has been simplified significantly. While brevity is good, the previous description likely provided valuable context for users about how this field is intended to be used.tests/test_validate/Snakefile (1)
30-35: Consider including 'tissue' in the explicit schema definition.The polars schema definition explicitly includes
sample,condition, andnproperties but not thetissueproperty which is also defined in the schema.schema={"sample": pl.String, "condition": pl.String, "n": pl.UInt8}, + schema={"sample": pl.String, "condition": pl.String, "n": pl.UInt8, "tissue": pl.String},snakemake/utils.py (4)
132-134: Use exception chaining for better traceback
Explicitly chaining the original exception will clarify the error’s origin and preserve the traceback.- except jsonschema.exceptions.ValidationError as e: - raise WorkflowError( - f"Error validating row {i} of data frame.", e - ) + except jsonschema.exceptions.ValidationError as e: + raise WorkflowError( + f"Error validating row {i} of data frame.", e + ) from e🧰 Tools
🪛 Ruff (0.8.2)
132-134: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
168-170: Preserve cause in Polars validation error
Chaining the exception helps analyze polars validation issues.- except jsonschema.exceptions.ValidationError as e: - raise WorkflowError( - f"Error validating row {i} of data frame.", e - ) + except jsonschema.exceptions.ValidationError as e: + raise WorkflowError( + f"Error validating row {i} of data frame.", e + ) from e🧰 Tools
🪛 Ruff (0.8.2)
168-170: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
206-208: Same exception chaining for repeated block
Applying the same fix ensures consistency in error handling throughout_validate_polars.- except jsonschema.exceptions.ValidationError as e: - raise WorkflowError( - f"Error validating row {i} of data frame.", e - ) + except jsonschema.exceptions.ValidationError as e: + raise WorkflowError( + f"Error validating row {i} of data frame.", e + ) from e🧰 Tools
🪛 Ruff (0.8.2)
206-208: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
219-226: Dictionary validation
Validation logic for dictionaries mirrors the approach for DataFrames, maintaining consistency. Consider applying the same exception-chaining pattern.- except jsonschema.exceptions.ValidationError as e: - raise WorkflowError("Error validating config file.", e) + except jsonschema.exceptions.ValidationError as e: + raise WorkflowError("Error validating config file.", e) from e🧰 Tools
🪛 Ruff (0.8.2)
224-224: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/test_validate/samples.tsvis excluded by!**/*.tsv
📒 Files selected for processing (9)
docs/project_info/codebase.rst(1 hunks)docs/project_info/contributing.rst(2 hunks)docs/snakefiles/configuration.rst(1 hunks)snakemake/assets/__init__.py(4 hunks)snakemake/dag.py(1 hunks)snakemake/report/html_reporter/data/packages.py(1 hunks)snakemake/utils.py(1 hunks)tests/test_validate/Snakefile(1 hunks)tests/test_validate/samples.schema.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/project_info/codebase.rst
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.py`: Do not try to improve formatting. Do not suggest ...
**/*.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.
snakemake/report/html_reporter/data/packages.pysnakemake/dag.pysnakemake/utils.pysnakemake/assets/__init__.py
🪛 Ruff (0.8.2)
snakemake/utils.py
132-134: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
168-170: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
206-208: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
224-224: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🔇 Additional comments (23)
snakemake/dag.py (1)
412-427: Good implementation of topological sorting for output storage!The change improves the organization of how outputs are stored by processing jobs in topological order (by levels). This ensures that dependent jobs have their outputs stored after their prerequisites, which:
- Prevents potential race conditions
- Creates a more deterministic execution order
- Follows the natural dependency flow of the workflow
The implementation correctly:
- Uses
toposortedto organize jobs by dependency levels- Converts the
tostorefunction to be async- Maintains the same logic for determining which files to store
tests/test_validate/samples.schema.yaml (1)
10-17: LGTM: Well-defined schema properties added.The new properties
nandtissueare well-defined with appropriate types, defaults, and clear descriptions. These additions enhance the schema while maintaining backward compatibility through sensible defaults.tests/test_validate/Snakefile (5)
4-4: LGTM: Added Polars import for additional testing.Adding Polars support expands the test coverage to different DataFrame implementations, which is a good testing practice.
11-20: LGTM: Good test coverage for dict validation.These tests thoroughly validate the schema's handling of dictionary data converted from DataFrame rows, including checking default values and explicit values.
22-27: LGTM: Well-structured DataFrame validation tests.The tests for Pandas DataFrame without index are well-structured and thorough.
42-48: Note the inconsistent validation parameter.Line 48 uses
set_default=Falsefor the LazyFrame validation, which differs from other validation calls. Consider documenting why this parameter is needed specifically for LazyFrames or standardize the approach across all validation calls.
51-57: LGTM: Enhanced existing test with assertions.Good job enhancing the existing Pandas DataFrame with index test to include assertions for the new schema properties.
docs/snakefiles/configuration.rst (1)
115-116: Expanded documentation forvalidatefunction
These lines clearly indicate thatsnakemake.utils.validatecan now handle configuration dictionaries, Pandas DataFrames, Polars DataFrames, and Polars LazyFrames, aligning with the updated implementation.docs/project_info/contributing.rst (2)
188-189: Adoption of Sphinx-recommended section structure
The reference to Sphinx’s guidelines clarifies the heading levels and improves the consistency of the documentation. Great addition!
228-229: Renamed Conda environment
Switching the environment tosnakemake_docsis a helpful change that more clearly indicates its purpose.snakemake/utils.py (4)
111-117: Introduction of_validate_recordfunction
The logic for applying defaults (if requested) and validating an individual record looks clean and correct.
118-131: Null filtering and DataFrame merging
The_validate_pandasfunction effectively filters out null values before validation and merges new columns when defaults are applied. Implementation appears solid.
150-167: _validate_polars for record-by-record validation
Adopting the same record-based validation logic as in_validate_pandasis consistent and helps maintain uniformity across data backends.
227-230: Logging for completed validation
These log messages help confirm which validation path was used, providing useful feedback for debugging.snakemake/report/html_reporter/data/packages.py (4)
41-105: Confirm correctness of newly added d3-related dependencies.
These references and pinned versions appear consistent with their known sources. However, please verify that each pinned dependency is indeed needed and that no extraneous references slipped in via tree-shaking.
106-136: Validate copied/adapted code references for vega.
Ensuring that thehashlru,d3-regression,regression, and other derived assets match the intended commit/version is critical. Double-check each pinned version for correctness and availability over time.
141-161: Audit newly introduced vega-lite dependencies.
The pinned vega-lite dependencies, such as@types-cloneandclone, look correct. However, a brief check to confirm their presence in the final bundle (i.e., not fully tree-shaken out) is recommended to avoid managing unused dependencies.
167-185: Recheck the vega-embed dependency suite.
Similar to the other dependencies, confirm that these pinned references (e.g.,fast-json-patch,semver) are valid over the long term, particularly given multiple versions from yarn.lock.snakemake/assets/__init__.py (5)
96-246: Validate newly introduced vega asset entries.
All these newly added asset definitions for vega and its d3-based dependencies look accurate at first glance. A quick verification that none of the pinned versions are broken or removed upstream would be worthwhile.
247-351: Double-check licensing for copied/adapted vega code.
Code references (e.g., fromhashlru,d3-regression,science) rely on older commit SHA pins or unusual license paths. Confirm that each reference remains online and that the licenses align with your distribution policy.
362-410: Ensure vega-lite dependencies still match the final bundled code.
Most references indicate partial inclusion or potential tree-shaking. Verify that each pinned reference (e.g.,clone,fast-deep-equal) maps to actual usage in the final artifact.
423-495: Confirm correctness of vega-embed references.
These pinned versions look carefully curated, particularly the singlesemverbuild. Double-check that no mismatched version remains from prior merges.
510-530:Details
✅ Verification successful
Run script to verify intact URLs for new asset references.
Below is a quick script to ensure that the newly introduced assets with static URLs do not yield errors (e.g., HTTP 404). This script skips placeholders (e.g.,{version}) since exact versions are embedded in the code.
🏁 Script executed:
#!/bin/bash # Description: Verify that newly introduced, non-placeholder URLs respond successfully. # Search for "url=" lines inside snakemake/assets/__init__.py # Exclude lines containing '{version}' placeholders, then perform a HEAD request on the rest. rg -o --line-number 'url="[^"]+"' snakemake/assets/__init__.py \ | sed -E 's/.*url="([^"]+)".*/\1/' \ | grep -v '{version}' \ | while read url; do echo "Checking $url..." curl -I --fail --silent "$url" || echo "URL check failed: $url" doneLength of output: 7414
Verified URL Integrity: All Asset URLs Return HTTP 200
The verification script confirmed that every non-placeholder URL in
snakemake/assets/__init__.py(lines 510–530) is intact and successfully returns an HTTP 200 response. No errors were found during the checks for:
- https://raw.githubusercontent.com/snakemake/snakemake/main/LICENSE.md
- https://raw.githubusercontent.com/pygments/pygments/master/LICENSE
- https://raw.githubusercontent.com/HarryStevens/d3-regression/e23d40a663dffba14b92cb42d9989de3a32894b5/LICENSE
- https://raw.githubusercontent.com/apache/commons-math/refs/tags/MATH_3_6_1/LICENSE.txt
- https://raw.githubusercontent.com/apache/commons-math/refs/tags/MATH_3_6_1/NOTICE.txt
- https://raw.githubusercontent.com/jonathantneal/array-flat-polyfill/362d855cb6ea2ef12f0676b116c5f9e4233b6f31/LICENSE.md
Please continue to use similar checks for any future modifications to asset references.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/tests.py (1)
2054-2057: Clarify the Windows skipping behavior.Although this test is labeled "OS agnostic," it is decorated with
@skip_on_windows. If the test can truly run on Windows, consider removing the skip decorator to provide broader test coverage. Otherwise, update the comment to note any platform-specific limitations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
snakemake/ruleinfo.py(2 hunks)snakemake/workflow.py(2 hunks)tests/tests.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- snakemake/workflow.py
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.py`: Do not try to improve formatting. Do not suggest ...
**/*.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.
snakemake/ruleinfo.pytests/tests.py
🔇 Additional comments (2)
snakemake/ruleinfo.py (2)
8-8: No issues with the new import.Bringing in
loggeraligns with the new logging usage below.
72-91:Details
❓ Verification inconclusive
Verify handling of
paramsand assure type consistency.
- The code assumes
paramsis a two-element tuple(positional, keyword). Ifparamsis missing or set to something else in the codebase, an error could occur at runtime."{**original_keyword, **modifier_keyword}"appropriately merges keyword dictionaries. If you want to track which keys were overwritten, consider adding a debug log for clarity.Use the following script to confirm that
paramsis always set as a two-element tuple throughout the codebase (i.e., never set toNoneor assigned an incompatible structure):
🏁 Script executed:
#!/bin/bash # Searches for assignments to 'params' to verify it always matches a two-element structure. fd -E "tests" -t f -e py --exec rg --multiline --multiline-dotall "params\s*=\s*\(?.*\)?"Length of output: 68643
Manual Verification Required: Validate
paramsStructure and Type Consistency
- Ensure that every assignment or modification to
paramsenforces it being a two‐element tuple—specifically, a pair where the first element holds positional parameters and the second holds keyword parameters.- Verify that no code path assigns an alternative structure (e.g., a list or a single value) to
params, as that could cause runtime errors.- Consider adding debug logging in the merging operation (i.e. when combining
{**original_keyword, **modifier_keyword}) to track which keyword entries get overwritten.- Since the automated script output was inconclusive, please manually review all
paramsassignments and usages across the codebase to ensure consistent type handling.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
snakemake/ruleinfo.py (1)
66-66: Avoid mutable default argument.Using mutable data structures like sets as default arguments can lead to unexpected behavior if the default value is modified between function calls.
- def apply_modifier( - self, - modifier, - rulename, - prefix_replacables={"input", "output", "log", "benchmark"}, - ): + def apply_modifier( + self, + modifier, + rulename, + prefix_replacables=None, + ): + """Update this ruleinfo with the given one (used for 'use rule' overrides).""" + if prefix_replacables is None: + prefix_replacables = {"input", "output", "log", "benchmark"}🧰 Tools
🪛 Ruff (0.8.2)
66-66: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
snakemake/ruleinfo.py(3 hunks)snakemake/workflow.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- snakemake/workflow.py
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.py`: Do not try to improve formatting. Do not suggest ...
**/*.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.
snakemake/ruleinfo.py
🪛 Ruff (0.8.2)
snakemake/ruleinfo.py
66-66: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
⏰ Context from checks skipped due to timeout of 90000ms (30)
- GitHub Check: testing (10, 3.12, bash)
- GitHub Check: testing (10, 3.11, bash)
- GitHub Check: testing (9, 3.12, bash)
- GitHub Check: testing (9, 3.11, bash)
- GitHub Check: testing (8, 3.12, bash)
- GitHub Check: testing (8, 3.11, bash)
- GitHub Check: testing (7, 3.12, bash)
- GitHub Check: testing (7, 3.11, bash)
- GitHub Check: testing (6, 3.12, bash)
- GitHub Check: testing (6, 3.11, bash)
- GitHub Check: testing (5, 3.12, bash)
- GitHub Check: testing (5, 3.11, bash)
- GitHub Check: testing (4, 3.12, bash)
- GitHub Check: testing-windows (10)
- GitHub Check: testing (4, 3.11, bash)
- GitHub Check: testing-windows (9)
- GitHub Check: testing (3, 3.12, bash)
- GitHub Check: testing-windows (8)
- GitHub Check: testing (3, 3.11, bash)
- GitHub Check: testing-windows (7)
- GitHub Check: testing (2, 3.12, bash)
- GitHub Check: testing-windows (6)
- GitHub Check: testing (2, 3.11, bash)
- GitHub Check: testing-windows (5)
- GitHub Check: testing-windows (4)
- GitHub Check: testing (1, 3.12, bash)
- GitHub Check: testing-windows (3)
- GitHub Check: testing-windows (2)
- GitHub Check: testing (1, 3.11, bash)
- GitHub Check: testing-windows (1)
🔇 Additional comments (3)
snakemake/ruleinfo.py (3)
8-8: Added logger import to support new parameter behavior logging.The import of
loggeris properly added to support the new warning logs for parameter overwriting.
75-92: Enhanced parameter handling to retain unmodified parameters.The implementation correctly handles both positional and keyword parameters when applying modifiers. The function now:
- Preserves original keyword parameters unless explicitly overwritten
- Logs a warning when positional parameters are being replaced
- Merges keyword parameters correctly
This aligns with the PR objective of keeping parameters that aren't redefined in the current rule.
63-66:Details
❓ Verification inconclusive
Added rulename parameter for improved logging context.
The new
rulenameparameter enhances logging by providing context about which rule's parameters are being overwritten.
🏁 Script executed:
#!/bin/bash # Check if the new parameter is consistently used across the codebase rg -p "apply_modifier\(\s*(?!.*rulename)" --type pyLength of output: 309
Enhanced Logging Context Verification
The addition of the
rulenameparameter insnakemake/ruleinfo.py(lines 63-66) is a solid improvement for logging clarity—it now clearly identifies which rule’s parameters are modified. Note, however, that our initial automated check ran into a regex parsing issue because look-around isn’t supported without enabling PCRE2.To verify that all calls to
apply_modifierare updated correctly, please execute the following revised command, which uses the--pcre2flag for proper regex support:#!/bin/bash # Verify that the rulename parameter is present in apply_modifier calls across the codebase rg --pcre2 "apply_modifier\(\s*[^)]*\brulename\b" --type pyOnce you've manually confirmed that every usage of
apply_modifierincludes the new parameter, no further changes are needed.🧰 Tools
🪛 Ruff (0.8.2)
66-66: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
…ems them when using 'with' directive (snakemake#3365) Keep parameters when using 'with' directive. If the included rule contains parameters that are not defined in the given rule, these parameters are inherited and only the newly specified parameter is overwritten. Related to issue snakemake#3338 * [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 --> - **New Features** - Enhanced workflow parameter management with improved merging of positional and keyword arguments. - Updated workflow rules for richer configuration and improved execution clarity. - **Tests** - Added an OS-agnostic test to validate workflow performance across different environments. - **Documentation** - Clarified documentation on parameter inheritance for better user understanding. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Johannes Hausmann <johannes.hausmann@posteo.de> Co-authored-by: Johannes Köster <johannes.koester@tu-dortmund.de> Co-authored-by: Matthias Peter <55617029+SrPeter128@users.noreply.github.com> Co-authored-by: Johannes Koester <johannes.koester@uni-due.de>
🤖 I have created a release *beep* *boop* --- ## [9.0.0](v8.30.0...v9.0.0) (2025-03-14) ### ⚠ BREAKING CHANGES * Logging refactor & add LoggerPluginInterface ([#3107](#3107)) ### Features * [#3412](#3412) - keep shadow folder of failed job if --keep-incomplete flag is set. ([#3430](#3430)) ([22978c3](22978c3)) * add flag --report-after-run to automatically generate the report after a successfull workflow run ([#3428](#3428)) ([b0a7f03](b0a7f03)) * add flatten function to IO utils ([#3424](#3424)) ([67fa392](67fa392)) * add helper functions to parse input files ([#2918](#2918)) ([63e45a7](63e45a7)) * Add option to print redacted file names ([#3089](#3089)) ([ba4d264](ba4d264)) * add support for validation of polars dataframe and lazyframe ([#3262](#3262)) ([c7473a6](c7473a6)) * added support for rendering dag with mermaid js ([#3409](#3409)) ([7bf8381](7bf8381)) * adding --replace-workflow-config to fully replace workflow configs (from config: directive) with --configfile, instead of merging them ([#3381](#3381)) ([47504a0](47504a0)) * Dynamic module name ([#3401](#3401)) ([024dc32](024dc32)) * Enable saving and reloading IOCache object ([#3386](#3386)) ([c935953](c935953)) * files added in rule params with workflow.source_path will be available in used containers ([#3385](#3385)) ([a6e45bf](a6e45bf)) * Fix keep_local in storage directive and more freedom over remote retrieval behaviour ([#3410](#3410)) ([67b4739](67b4739)) * inherit parameters of use rule and extend/replace individual items them when using 'with' directive ([#3365](#3365)) ([93e4b92](93e4b92)) * Logging refactor & add LoggerPluginInterface ([#3107](#3107)) ([86f1d6e](86f1d6e)) * Maximal file size for checksums ([#3368](#3368)) ([b039f8a](b039f8a)) * Modernize package configuration using Pixi ([#3369](#3369)) ([77992d8](77992d8)) * multiext support for named input/output ([#3372](#3372)) ([05e1378](05e1378)) * optionally auto-group jobs via temp files in case of remote execution ([#3378](#3378)) ([cc9bba2](cc9bba2)) ### Bug Fixes * `--delete-all-output` ignores `--dry-run` ([#3265](#3265)) ([23fef82](23fef82)) * 3342 faster touch runs and warning messages for non-existing files ([#3398](#3398)) ([cd9c3c3](cd9c3c3)) * add default value to max-jobs-per-timespan ([#3043](#3043)) ([2959abe](2959abe)) * checkpoints inside modules are overwritten ([#3359](#3359)) ([fba3ac7](fba3ac7)) * Convert Path to IOFile ([#3405](#3405)) ([c58684c](c58684c)) * Do not perform storage object cleanup with --keep-storage-local-copies set ([#3358](#3358)) ([9a6d14b](9a6d14b)) * edgecases of source deployment in case of remote execution ([#3396](#3396)) ([5da13be](5da13be)) * enhance error message formatting for strict DAG-building mode ([#3376](#3376)) ([a1c39ee](a1c39ee)) * fix bug in checkpoint handling that led to exceptions in case checkpoint output was missing upon rerun ([#3423](#3423)) ([8cf4a2f](8cf4a2f)) * force check all required outputs ([#3341](#3341)) ([495a4e7](495a4e7)) * group job formatting ([#3442](#3442)) ([f0b10a3](f0b10a3)) * in remote jobs, upload storage in topological order such that modification dates are preserved (e.g. in case of group jobs) ([#3377](#3377)) ([eace08f](eace08f)) * only skip eval when resource depends on input ([#3374](#3374)) ([4574c92](4574c92)) * Prevent execution of conda in apptainer when not explicitly requested in software deployment method ([#3388](#3388)) ([c43c5c0](c43c5c0)) * print filenames with quotes around them in RuleException ([#3269](#3269)) ([6baeda5](6baeda5)) * Re-evaluation of free resources ([#3399](#3399)) ([6371293](6371293)) * ReadTheDocs layout issue due to src directory change ([#3419](#3419)) ([695b127](695b127)) * robustly escaping quotes in generated bash scripts (v2) ([#3297](#3297)) ([#3389](#3389)) ([58720bd](58720bd)) * Show apptainer image URL in snakemake report ([#3407](#3407)) ([45f0450](45f0450)) * Update ReadTheDocs configuration for documentation build to use Pixi ([#3433](#3433)) ([3f227a6](3f227a6)) ### Documentation * Add pixi setup instructions to general use tutorial ([#3382](#3382)) ([115e81b](115e81b)) * fix contribution section heading levels, fix docs testing setup order ([#3360](#3360)) ([051dc53](051dc53)) * fix link to github.com/snakemake/poetry-snakemake-plugin ([#3436](#3436)) ([ec6d97c](ec6d97c)) * fix quoting ([#3394](#3394)) ([b40f599](b40f599)) * fix rerun-triggers default ([#3403](#3403)) ([4430e23](4430e23)) * fix typo 'safe' -> 'save' ([#3384](#3384)) ([7755861](7755861)) * mention code formatting in the contribution section ([#3431](#3431)) ([e8682b7](e8682b7)) * remove duplicated 'functions'. ([#3356](#3356)) ([7c595db](7c595db)) * update broken links documentation ([#3437](#3437)) ([e3d0d88](e3d0d88)) * Updating contributing guidelines with new pixi dev setup ([#3415](#3415)) ([8e95a12](8e95a12)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: snakemake-bot <snakemake-bot-admin@googlegroups.com>
…andle not only params but also input, output, resources, config, and so on
…andle not only params but also input, output, resources, config, and so on
…ems them when using 'with' directive (snakemake#3365) Keep parameters when using 'with' directive. If the included rule contains parameters that are not defined in the given rule, these parameters are inherited and only the newly specified parameter is overwritten. Related to issue snakemake#3338 ### QC * [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 - **New Features** - Enhanced workflow parameter management with improved merging of positional and keyword arguments. - Updated workflow rules for richer configuration and improved execution clarity. - **Tests** - Added an OS-agnostic test to validate workflow performance across different environments. - **Documentation** - Clarified documentation on parameter inheritance for better user understanding. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Johannes Hausmann <johannes.hausmann@posteo.de> Co-authored-by: Johannes Köster <johannes.koester@tu-dortmund.de> Co-authored-by: Matthias Peter <55617029+SrPeter128@users.noreply.github.com> Co-authored-by: Johannes Koester <johannes.koester@uni-due.de>
🤖 I have created a release *beep* *boop* --- ## [9.0.0](snakemake/snakemake@v8.30.0...v9.0.0) (2025-03-14) ### ⚠ BREAKING CHANGES * Logging refactor & add LoggerPluginInterface ([snakemake#3107](snakemake#3107)) ### Features * [snakemake#3412](snakemake#3412) - keep shadow folder of failed job if --keep-incomplete flag is set. ([snakemake#3430](snakemake#3430)) ([22978c3](snakemake@22978c3)) * add flag --report-after-run to automatically generate the report after a successfull workflow run ([snakemake#3428](snakemake#3428)) ([b0a7f03](snakemake@b0a7f03)) * add flatten function to IO utils ([snakemake#3424](snakemake#3424)) ([67fa392](snakemake@67fa392)) * add helper functions to parse input files ([snakemake#2918](snakemake#2918)) ([63e45a7](snakemake@63e45a7)) * Add option to print redacted file names ([snakemake#3089](snakemake#3089)) ([ba4d264](snakemake@ba4d264)) * add support for validation of polars dataframe and lazyframe ([snakemake#3262](snakemake#3262)) ([c7473a6](snakemake@c7473a6)) * added support for rendering dag with mermaid js ([snakemake#3409](snakemake#3409)) ([7bf8381](snakemake@7bf8381)) * adding --replace-workflow-config to fully replace workflow configs (from config: directive) with --configfile, instead of merging them ([snakemake#3381](snakemake#3381)) ([47504a0](snakemake@47504a0)) * Dynamic module name ([snakemake#3401](snakemake#3401)) ([024dc32](snakemake@024dc32)) * Enable saving and reloading IOCache object ([snakemake#3386](snakemake#3386)) ([c935953](snakemake@c935953)) * files added in rule params with workflow.source_path will be available in used containers ([snakemake#3385](snakemake#3385)) ([a6e45bf](snakemake@a6e45bf)) * Fix keep_local in storage directive and more freedom over remote retrieval behaviour ([snakemake#3410](snakemake#3410)) ([67b4739](snakemake@67b4739)) * inherit parameters of use rule and extend/replace individual items them when using 'with' directive ([snakemake#3365](snakemake#3365)) ([93e4b92](snakemake@93e4b92)) * Logging refactor & add LoggerPluginInterface ([snakemake#3107](snakemake#3107)) ([86f1d6e](snakemake@86f1d6e)) * Maximal file size for checksums ([snakemake#3368](snakemake#3368)) ([b039f8a](snakemake@b039f8a)) * Modernize package configuration using Pixi ([snakemake#3369](snakemake#3369)) ([77992d8](snakemake@77992d8)) * multiext support for named input/output ([snakemake#3372](snakemake#3372)) ([05e1378](snakemake@05e1378)) * optionally auto-group jobs via temp files in case of remote execution ([snakemake#3378](snakemake#3378)) ([cc9bba2](snakemake@cc9bba2)) ### Bug Fixes * `--delete-all-output` ignores `--dry-run` ([snakemake#3265](snakemake#3265)) ([23fef82](snakemake@23fef82)) * 3342 faster touch runs and warning messages for non-existing files ([snakemake#3398](snakemake#3398)) ([cd9c3c3](snakemake@cd9c3c3)) * add default value to max-jobs-per-timespan ([snakemake#3043](snakemake#3043)) ([2959abe](snakemake@2959abe)) * checkpoints inside modules are overwritten ([snakemake#3359](snakemake#3359)) ([fba3ac7](snakemake@fba3ac7)) * Convert Path to IOFile ([snakemake#3405](snakemake#3405)) ([c58684c](snakemake@c58684c)) * Do not perform storage object cleanup with --keep-storage-local-copies set ([snakemake#3358](snakemake#3358)) ([9a6d14b](snakemake@9a6d14b)) * edgecases of source deployment in case of remote execution ([snakemake#3396](snakemake#3396)) ([5da13be](snakemake@5da13be)) * enhance error message formatting for strict DAG-building mode ([snakemake#3376](snakemake#3376)) ([a1c39ee](snakemake@a1c39ee)) * fix bug in checkpoint handling that led to exceptions in case checkpoint output was missing upon rerun ([snakemake#3423](snakemake#3423)) ([8cf4a2f](snakemake@8cf4a2f)) * force check all required outputs ([snakemake#3341](snakemake#3341)) ([495a4e7](snakemake@495a4e7)) * group job formatting ([snakemake#3442](snakemake#3442)) ([f0b10a3](snakemake@f0b10a3)) * in remote jobs, upload storage in topological order such that modification dates are preserved (e.g. in case of group jobs) ([snakemake#3377](snakemake#3377)) ([eace08f](snakemake@eace08f)) * only skip eval when resource depends on input ([snakemake#3374](snakemake#3374)) ([4574c92](snakemake@4574c92)) * Prevent execution of conda in apptainer when not explicitly requested in software deployment method ([snakemake#3388](snakemake#3388)) ([c43c5c0](snakemake@c43c5c0)) * print filenames with quotes around them in RuleException ([snakemake#3269](snakemake#3269)) ([6baeda5](snakemake@6baeda5)) * Re-evaluation of free resources ([snakemake#3399](snakemake#3399)) ([6371293](snakemake@6371293)) * ReadTheDocs layout issue due to src directory change ([snakemake#3419](snakemake#3419)) ([695b127](snakemake@695b127)) * robustly escaping quotes in generated bash scripts (v2) ([snakemake#3297](snakemake#3297)) ([snakemake#3389](snakemake#3389)) ([58720bd](snakemake@58720bd)) * Show apptainer image URL in snakemake report ([snakemake#3407](snakemake#3407)) ([45f0450](snakemake@45f0450)) * Update ReadTheDocs configuration for documentation build to use Pixi ([snakemake#3433](snakemake#3433)) ([3f227a6](snakemake@3f227a6)) ### Documentation * Add pixi setup instructions to general use tutorial ([snakemake#3382](snakemake#3382)) ([115e81b](snakemake@115e81b)) * fix contribution section heading levels, fix docs testing setup order ([snakemake#3360](snakemake#3360)) ([051dc53](snakemake@051dc53)) * fix link to github.com/snakemake/poetry-snakemake-plugin ([snakemake#3436](snakemake#3436)) ([ec6d97c](snakemake@ec6d97c)) * fix quoting ([snakemake#3394](snakemake#3394)) ([b40f599](snakemake@b40f599)) * fix rerun-triggers default ([snakemake#3403](snakemake#3403)) ([4430e23](snakemake@4430e23)) * fix typo 'safe' -> 'save' ([snakemake#3384](snakemake#3384)) ([7755861](snakemake@7755861)) * mention code formatting in the contribution section ([snakemake#3431](snakemake#3431)) ([e8682b7](snakemake@e8682b7)) * remove duplicated 'functions'. ([snakemake#3356](snakemake#3356)) ([7c595db](snakemake@7c595db)) * update broken links documentation ([snakemake#3437](snakemake#3437)) ([e3d0d88](snakemake@e3d0d88)) * Updating contributing guidelines with new pixi dev setup ([snakemake#3415](snakemake#3415)) ([8e95a12](snakemake@8e95a12)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: snakemake-bot <snakemake-bot-admin@googlegroups.com>
Keep parameters when using 'with' directive.
If the included rule contains parameters that are not defined in the given rule, these parameters are inherited and only the newly specified parameter is overwritten.
Related to issue #3338
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
New Features
Tests
Documentation