feat: Dynamic module name#3401
Conversation
📝 WalkthroughWalkthroughThe changes add multiple new Snakefiles and configuration files across various test directories that define workflows using dynamic module loading, aliasing, and prefix replacement features. New module workflows include rules that chain outputs, invoke functions returning fixed numeric values, and perform assertions. Several new test functions have been added to exercise scenarios such as dynamic, semi-dynamic, and explicitly named modules. In addition, documentation is updated to explain dynamic module loading, and source code enhancements improve module parsing and workflow handling by allowing optional module names and enhanced error reporting. Changes
Sequence Diagram(s)sequenceDiagram
participant S as Snakefile
participant P as Parser
participant M as Module
participant W as Workflow
participant R as Rule Engine
participant T as Test Suite
S->>P: Read module declaration (module, name, config)
P->>M: Initialize module via start() & name() methods
M-->>P: Return module with name attribute and defined rules
P->>W: Pass module object to workflow
W->>R: Import and alias module rules with prefix replacement
R-->>W: Aggregate rules and build execution plan
W->>T: Execute workflow (invoking shell commands, functions like some_func())
T->>W: Validate outputs and assert expected values
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
🪧 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 (5)
snakemake/parser.py (1)
954-954: Unnecessary f-stringThis line uses an f-string without any placeholders, which is unnecessary and can be simplified.
- yield f"workflow.module(", token + yield "workflow.module(", token🧰 Tools
🪛 Ruff (0.8.2)
954-954: f-string without any placeholders
Remove extraneous
fprefix(F541)
tests/test_modules_name/config/config.yaml (1)
1-1: YAML configuration fileThe configuration contains the minimal test data needed for the test case. This is appropriate for a test configuration.
The file is missing a newline at the end, which is flagged by YAMLlint. While this won't affect functionality, it's good practice to include a newline at the end of YAML files.
-test: 1 +test: 1 +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: no new line character at the end of file
(new-line-at-end-of-file)
tests/test_modules_no_name/config/config.yaml (1)
1-1: YAML configuration fileThe configuration contains the minimal test data needed for the test case. This is appropriate for a test configuration.
The file is missing a newline at the end, which is flagged by YAMLlint. While this won't affect functionality, it's good practice to include a newline at the end of YAML files.
-test: 1 +test: 1 +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: no new line character at the end of file
(new-line-at-end-of-file)
tests/test_modules_dynamic/config/config.yaml (1)
1-1: YAML configuration fileThe configuration contains the minimal test data needed for the test case. This is appropriate for a test configuration.
The file is missing a newline at the end, which is flagged by YAMLlint. While this won't affect functionality, it's good practice to include a newline at the end of YAML files.
-test: 1 +test: 1 +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: no new line character at the end of file
(new-line-at-end-of-file)
tests/test_modules_two_names/config/config.yaml (1)
1-1: Simple configuration file for the test case.The YAML file contains a single key-value pair which is sufficient for testing purposes.
Consider adding a newline at the end of the file to comply with standard file formatting practices.
-test: 1 +test: 1 +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: 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 ignored due to path filters (5)
tests/test_modules_dynamic/expected-results/results/testmodule/b1/test.outis excluded by!**/*.outtests/test_modules_dynamic/expected-results/results/testmodule/b2/test.outis excluded by!**/*.outtests/test_modules_name/expected-results/results/testmodule/b/test.outis excluded by!**/*.outtests/test_modules_no_name/expected-results/results/testmodule/b/test.outis excluded by!**/*.outtests/test_modules_two_names/expected-results/results/testmodule/b/test.outis excluded by!**/*.out
📒 Files selected for processing (16)
snakemake/parser.py(5 hunks)snakemake/workflow.py(1 hunks)tests/test_modules_dynamic/Snakefile(1 hunks)tests/test_modules_dynamic/config/config.yaml(1 hunks)tests/test_modules_dynamic/module1/Snakefile(1 hunks)tests/test_modules_dynamic/module2/Snakefile(1 hunks)tests/test_modules_name/Snakefile(1 hunks)tests/test_modules_name/config/config.yaml(1 hunks)tests/test_modules_name/module-test/Snakefile(1 hunks)tests/test_modules_no_name/Snakefile(1 hunks)tests/test_modules_no_name/config/config.yaml(1 hunks)tests/test_modules_no_name/module-test/Snakefile(1 hunks)tests/test_modules_two_names/Snakefile(1 hunks)tests/test_modules_two_names/config/config.yaml(1 hunks)tests/test_modules_two_names/module-test/Snakefile(1 hunks)tests/tests.py(1 hunks)
🧰 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.
tests/tests.pysnakemake/parser.pysnakemake/workflow.py
🪛 YAMLlint (1.35.1)
tests/test_modules_no_name/config/config.yaml
[error] 1-1: no new line character at the end of file
(new-line-at-end-of-file)
tests/test_modules_dynamic/config/config.yaml
[error] 1-1: no new line character at the end of file
(new-line-at-end-of-file)
tests/test_modules_name/config/config.yaml
[error] 1-1: no new line character at the end of file
(new-line-at-end-of-file)
tests/test_modules_two_names/config/config.yaml
[error] 1-1: no new line character at the end of file
(new-line-at-end-of-file)
🪛 Ruff (0.8.2)
snakemake/parser.py
954-954: f-string without any placeholders
Remove extraneous f prefix
(F541)
⏰ Context from checks skipped due to timeout of 90000ms (25)
- 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-windows (10)
- GitHub Check: testing (5, 3.11, bash)
- GitHub Check: testing (4, 3.12, bash)
- GitHub Check: testing-windows (7)
- GitHub Check: testing (3, 3.12, bash)
- GitHub Check: testing-windows (6)
- GitHub Check: testing (3, 3.11, bash)
- GitHub Check: testing-windows (5)
- GitHub Check: testing (2, 3.12, bash)
- GitHub Check: testing (2, 3.11, bash)
- GitHub Check: testing-windows (3)
- GitHub Check: testing (1, 3.12, bash)
- GitHub Check: testing (1, 3.11, bash)
- GitHub Check: testing-windows (1)
🔇 Additional comments (46)
snakemake/parser.py (6)
870-875: Implementation of ModuleKeywordState methodsThe added
startandendmethods in theModuleKeywordStateclass provide the necessary infrastructure for handling module names more structurally. Thestartmethod yields thekeyword=which will be used to identify module attributes, while theendmethod properly closes the statement with a comma.
877-879: New ModuleName classThis new class inherits from
ModuleKeywordStateand provides a dedicated way to handle module names, improving the structure of the code. This is a good practice as it follows the same pattern used for other module attributes.
927-927: Module name trackingAdding the
modulenameattribute to theModuleclass enables tracking the module name, which is essential for supporting the dynamic module name feature described in the PR objectives.
932-939: Improved module name handlingThis code now properly yields the module name if it exists or raises an error if it doesn't. This ensures modules always have a name, either explicitly provided or generated dynamically.
949-950: Setting module nameThe code now stores the module name in the
self.modulenameattribute instead of directly yielding it, which allows for more flexible handling of module names.
969-976: Ambiguous module name checkGood error checking for ambiguous module names when a name is provided both after the module keyword and with the name keyword. This prevents confusing configurations and makes the API more robust.
tests/test_modules_two_names/Snakefile (4)
7-14: Module definition with explicit name looks good.This module definition correctly uses the new
nameparameter to explicitly set the module name to "test". The configuration and prefix replacement are properly configured.
17-17: Rule import using named module.The wildcard rule import correctly references the named module.
20-22: Rule definition is correct.The "all" rule properly defines the expected output files from the module.
25-25: Module function access assertion.The assertion correctly verifies that the function from the module returns the expected value, validating that module functions can be accessed after naming the module.
tests/test_modules_dynamic/Snakefile (4)
6-14: Dynamic module name implementation looks good.This is the core feature being added - dynamically setting module names in a loop. The implementation correctly:
- Iterates through a list of module names
- Creates a module for each name using the variable
mod_name- Sets the snakefile path dynamically based on the module name
- Passes the configuration and prefix replacement options
This is a well-implemented test case for the dynamic module name feature.
17-18: Rule imports correctly reference dynamically named modules.The wildcard rule imports correctly reference each module by its dynamic name.
20-23: 'all' rule correctly references output from both modules.The rule properly defines the expected output files from both dynamically named modules.
26-27: Module function access assertions for dynamic modules.The assertions correctly verify that functions from each dynamically named module return their expected values, validating that the dynamic naming works properly.
tests/test_modules_no_name/Snakefile (4)
7-13: Module definition without explicit name.This module definition maintains backward compatibility by not including a name parameter. The module is still accessible via the default name "test".
16-16: Rule import using the default module name.The wildcard rule import correctly references the unnamed module using the default name "test".
19-21: 'all' rule correctly defines inputs.The rule properly defines the expected output files from the unnamed module.
24-24: Module function access assertion using default name.The assertion correctly verifies that the function from the unnamed module returns the expected value, confirming backward compatibility.
tests/test_modules_name/Snakefile (6)
1-2: Clear shell configuration setup.The executable is properly set to "bash" which ensures consistent shell behavior across different environments.
4-4: Good practice using a local config file.Using a configuration file is a good practice for externalizing parameters and making the workflow more reusable.
7-14: Clear module definition with a proper name.The module is correctly defined with a name 'test', which is the core feature being tested. The module includes appropriate configuration and output path remapping.
17-17: Effective use of wildcard import with prefix.Using
use rule * from test as test_*is a good way to import and rename all rules from the module, preventing naming conflicts.
20-23: Rule definition with proper multiext usage.The rule correctly uses multiext with expand to generate file targets with multiple extensions.
25-25: Functional validation of module import.The assertion confirms that the function from the module was properly imported and can be accessed through the module namespace.
snakemake/workflow.py (1)
2170-2189: Properly implemented optional name parameter for module method.The change makes the
nameparameter optional with a default value ofNone, improving flexibility when defining modules. This implementation maintains backward compatibility while enabling dynamic module naming capabilities.The instantiation of
ModuleInfois correctly updated to pass thenameparameter, ensuring proper integration with the rest of the module system.tests/tests.py (4)
1439-1444: Good test case for named modules.The test ensures that modules can be properly defined with explicit names and function correctly.
1446-1452: Appropriate negative test for unnamed modules.This test correctly verifies that the system rejects invalid scenarios where a module has no name parameter. The
shouldfail=Trueindicates that this test is expected to fail, which is the correct behavior.
1454-1460: Valid test for module name conflicts.This test case ensures that the system properly handles the error case of having two modules with the same name, maintaining the integrity of the module system.
1462-1464: Test for dynamic module functionality.This test validates the dynamic module naming feature, ensuring it works correctly when targeting "all" outputs.
tests/test_modules_dynamic/module1/Snakefile (7)
1-2: Good use of configuration with appropriate comment.The configfile directive with the explanatory comment about it being ignored during module import provides clarity about the expected behavior.
4-6: Simple utility function for testing.The
some_func()function provides a straightforward way to test module function access in the parent workflow.
8-13: Well-designed rule with proper temp file designation.The rule
acorrectly uses thetemp()directive for intermediate files that don't need to be kept after the workflow completes, which is a good practice for managing disk space.
15-22: Clear input/output relationship with proper shell command.Rule
bproperly references the output of ruleavia therulesnamespace and uses a simple shell command to process the data.
24-31: Well-structured rule with clear naming.Rule
c_tsvmaintains the pattern of referencing previous rules' outputs and has a descriptive name that indicates its purpose.
33-36: Effective use of rule inheritance.Using
use rule c_tsv as c_txt with:demonstrates a good practice of rule reuse with minimal changes, avoiding code duplication.
38-40: Well-designed aggregation rule.The
allrule properly collects outputs from previous rules, ensuring that the workflow can be run with a simple target.tests/test_modules_dynamic/module2/Snakefile (7)
1-2: Well-structured configuration setup.The configfile declaration with the comment explaining that it doesn't exist and should be ignored during module import is a good practice for testing modules.
4-6: LGTM: Function definition is clear.The function
some_func()returns a constant value of 25, which appears to be deliberately different from the value in other test modules (which return 15) for testing purposes.
8-13: Well-designed rule with appropriate temporary output.The rule correctly uses the
temp()directive for intermediate files, which is a good practice in Snakemake workflows. The shell command references the configuration value appropriately.
15-22: LGTM: Rule dependency correctly established.Rule
bproperly references the output of ruleaas its input using theexpand()function, establishing a clear workflow dependency.
24-31: Rule follows consistent pattern.Rule
c_tsvcorrectly builds on the dependency chain, using ruleb's output as input. The pattern of usingcatto pass file contents through the workflow is consistent.
33-36: Effective use of rule reuse.The rule reuse syntax is correctly applied to create a variant of
c_tsvwith a different file extension.
38-41: Appropriate definition of target rule.Rule
allproperly aggregates outputs from rulec_tsvto define the workflow's default target.tests/test_modules_two_names/module-test/Snakefile (1)
1-41: LGTM: Well-structured test module.This Snakefile follows the same pattern as the other test modules but with a different module path structure. The function
some_func()returns 15 instead of 25 as in module2, which appears to be intentional for testing different module configurations.The workflow structure with rules a, b, c_tsv, and their dependencies is well-organized and follows Snakemake best practices.
tests/test_modules_name/module-test/Snakefile (1)
1-41: LGTM: Consistent test module definition.This test module is identical to the one in test_modules_two_names, which is expected as they appear to be testing different aspects of module naming functionality with the same underlying workflow structure.
The workflow design with clear rule dependencies and the consistent use of
expand()for generating file paths is well-implemented.tests/test_modules_no_name/module-test/Snakefile (1)
1-41: LGTM: Appropriate test structure for module functionality.This module is consistent with the other test modules, providing a uniform test structure across different module naming scenarios. The directory structure (test_modules_no_name) suggests this is testing the behavior when no explicit name is provided.
The workflow logic follows the same pattern as the other test modules, which enables consistent testing of the dynamic module name feature across different conditions.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
snakemake/workflow.py (1)
2284-2299: Improved module resolution with fallback to frame globals.This enhancement allows modules to be referenced by variables in the current scope rather than only direct string references. The code now checks if the module name exists as a variable in the calling frame's globals when it's not found in the registered modules.
The dynamic name_modifier handling also complements this feature by supporting wildcard suffixes.
Consider updating the exception to use the
from Nonesyntax to better differentiate it from potential errors in the exception handling itself:else: raise WorkflowError( "Module {} has not been registered with 'module' statement before using it in 'use rule' statement.".format( from_module ) - ) + ) from None🧰 Tools
🪛 Ruff (0.8.2)
2295-2299: 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 selected for processing (3)
snakemake/workflow.py(2 hunks)tests/test_modules_dynamic/Snakefile(1 hunks)tests/tests.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/test_modules_dynamic/Snakefile
- tests/tests.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/workflow.py
🪛 Ruff (0.8.2)
snakemake/workflow.py
2295-2299: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
⏰ Context from checks skipped due to timeout of 90000ms (36)
- GitHub Check: tests (10, windows-latest, py312, bash)
- GitHub Check: tests (10, windows-latest, py311, bash)
- GitHub Check: tests (10, ubuntu-latest, py312, bash)
- GitHub Check: tests (10, ubuntu-latest, py311, bash)
- GitHub Check: tests (9, windows-latest, py312, bash)
- GitHub Check: tests (9, windows-latest, py311, bash)
- GitHub Check: tests (9, ubuntu-latest, py312, bash)
- GitHub Check: tests (8, windows-latest, py312, bash)
- GitHub Check: tests (8, windows-latest, py311, bash)
- GitHub Check: tests (8, ubuntu-latest, py312, bash)
- GitHub Check: tests (8, ubuntu-latest, py311, bash)
- GitHub Check: tests (7, windows-latest, py312, bash)
- GitHub Check: tests (7, windows-latest, py311, bash)
- GitHub Check: tests (7, ubuntu-latest, py312, bash)
- GitHub Check: tests (7, ubuntu-latest, py311, bash)
- GitHub Check: tests (6, windows-latest, py312, bash)
- GitHub Check: tests (6, windows-latest, py311, bash)
- GitHub Check: tests (6, ubuntu-latest, py312, bash)
- GitHub Check: tests (6, ubuntu-latest, py311, bash)
- GitHub Check: tests (5, windows-latest, py312, bash)
- GitHub Check: tests (5, windows-latest, py311, bash)
- GitHub Check: tests (5, ubuntu-latest, py312, bash)
- GitHub Check: tests (5, ubuntu-latest, py311, bash)
- GitHub Check: tests (4, windows-latest, py312, bash)
- GitHub Check: tests (4, windows-latest, py311, bash)
- GitHub Check: tests (4, ubuntu-latest, py312, bash)
- GitHub Check: tests (4, ubuntu-latest, py311, bash)
- GitHub Check: tests (3, windows-latest, py312, bash)
- GitHub Check: tests (3, windows-latest, py311, bash)
- GitHub Check: tests (3, ubuntu-latest, py312, bash)
- GitHub Check: tests (3, ubuntu-latest, py311, bash)
- GitHub Check: tests (2, windows-latest, py311, bash)
- GitHub Check: tests (2, ubuntu-latest, py312, bash)
- GitHub Check: tests (2, ubuntu-latest, py311, bash)
- GitHub Check: tests (1, windows-latest, py312, bash)
- GitHub Check: tests (1, windows-latest, py311, bash)
🔇 Additional comments (1)
snakemake/workflow.py (1)
2249-2251: Making thenameparameter optional supports dynamic module naming.This change allows for more flexible module usage by making the
nameparameter optional with a default value ofNone. This aligns with the PR objective of supporting dynamic module names.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/test_modules_dynamic_no_as/config/config.yaml (1)
1-1: Add a newline at the end of the file.The file is missing a trailing newline, which is a common YAML convention.
-test: 1 +test: 1 +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: no new line character at the end of file
(new-line-at-end-of-file)
snakemake/workflow.py (1)
2284-2300: Improve exception handling withraise ... from errpattern.The error handling logic for retrieving modules from the current frame's globals works correctly, but the exception handling should follow Python's best practices by using the
raise ... from errpattern to maintain the exception context.except KeyError: from inspect import currentframe if from_module in currentframe().f_back.f_globals: module = self.modules[ currentframe().f_back.f_globals[from_module] ] if name_modifier is not None: if name_modifier.endswith("*"): modifier = f"{currentframe().f_back.f_globals[name_modifier[:-1]]}*" else: modifier = currentframe().f_back.f_globals[name_modifier] else: - raise WorkflowError( + raise WorkflowError( "Module {} has not been registered with 'module' statement before using it in 'use rule' statement.".format( from_module ) - ) + ) from None🧰 Tools
🪛 Ruff (0.8.2)
2296-2300: 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 (2)
tests/test_modules_dynamic_no_as/expected-results/results/testmodule/b1/test.outis excluded by!**/*.outtests/test_modules_dynamic_no_as/expected-results/results/testmodule/b2/test.outis excluded by!**/*.out
📒 Files selected for processing (6)
snakemake/workflow.py(2 hunks)tests/test_modules_dynamic_no_as/Snakefile(1 hunks)tests/test_modules_dynamic_no_as/config/config.yaml(1 hunks)tests/test_modules_dynamic_no_as/module1/Snakefile(1 hunks)tests/test_modules_dynamic_no_as/module2/Snakefile(1 hunks)tests/tests.py(1 hunks)
🧰 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.
tests/tests.pysnakemake/workflow.py
🪛 Ruff (0.8.2)
snakemake/workflow.py
2296-2300: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🪛 YAMLlint (1.35.1)
tests/test_modules_dynamic_no_as/config/config.yaml
[error] 1-1: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (40)
- GitHub Check: tests (10, windows-latest, py312, bash)
- GitHub Check: tests (10, windows-latest, py311, bash)
- GitHub Check: tests (10, ubuntu-latest, py312, bash)
- GitHub Check: tests (10, ubuntu-latest, py311, bash)
- GitHub Check: tests (9, windows-latest, py312, bash)
- GitHub Check: tests (9, windows-latest, py311, bash)
- GitHub Check: tests (9, ubuntu-latest, py312, bash)
- GitHub Check: tests (9, ubuntu-latest, py311, bash)
- GitHub Check: tests (8, windows-latest, py312, bash)
- GitHub Check: tests (8, windows-latest, py311, bash)
- GitHub Check: tests (8, ubuntu-latest, py312, bash)
- GitHub Check: tests (8, ubuntu-latest, py311, bash)
- GitHub Check: tests (7, windows-latest, py312, bash)
- GitHub Check: tests (7, windows-latest, py311, bash)
- GitHub Check: tests (7, ubuntu-latest, py312, bash)
- GitHub Check: tests (7, ubuntu-latest, py311, bash)
- GitHub Check: tests (6, windows-latest, py312, bash)
- GitHub Check: tests (6, windows-latest, py311, bash)
- GitHub Check: tests (6, ubuntu-latest, py312, bash)
- GitHub Check: tests (6, ubuntu-latest, py311, bash)
- GitHub Check: tests (5, windows-latest, py312, bash)
- GitHub Check: tests (5, windows-latest, py311, bash)
- GitHub Check: tests (5, ubuntu-latest, py312, bash)
- GitHub Check: tests (5, ubuntu-latest, py311, bash)
- GitHub Check: tests (4, windows-latest, py312, bash)
- GitHub Check: tests (4, windows-latest, py311, bash)
- GitHub Check: tests (4, ubuntu-latest, py312, bash)
- GitHub Check: tests (4, ubuntu-latest, py311, bash)
- GitHub Check: tests (3, windows-latest, py312, bash)
- GitHub Check: tests (3, windows-latest, py311, bash)
- GitHub Check: tests (3, ubuntu-latest, py312, bash)
- GitHub Check: tests (3, ubuntu-latest, py311, bash)
- GitHub Check: tests (2, windows-latest, py312, bash)
- GitHub Check: tests (2, windows-latest, py311, bash)
- GitHub Check: tests (2, ubuntu-latest, py312, bash)
- GitHub Check: tests (2, ubuntu-latest, py311, bash)
- GitHub Check: tests (1, windows-latest, py312, bash)
- GitHub Check: tests (1, windows-latest, py311, bash)
- GitHub Check: tests (1, ubuntu-latest, py312, bash)
- GitHub Check: tests (1, ubuntu-latest, py311, bash)
🔇 Additional comments (19)
tests/test_modules_dynamic_no_as/Snakefile (2)
6-15: The dynamic module name implementation looks good.The implementation correctly iterates over module names and creates modules with dynamic names. The module configuration is well-structured with appropriate settings.
24-25: The assertions properly verify the dynamic module functionality.The assertions confirm that functions from dynamically created modules can be properly accessed and return expected values.
tests/tests.py (1)
1440-1469: Good test coverage for the new module naming functionality.The added tests appropriately cover different scenarios:
- Modules with explicit names
- Modules without names (expected to fail)
- Modules with multiple names (expected to fail)
- Dynamic module creation with and without "as" syntax
This comprehensive test coverage helps ensure the feature works correctly across various use cases.
snakemake/workflow.py (2)
2249-2251: Good implementation of optional module name parameter.Making the
nameparameter optional is the core change needed to support dynamic module names, and it's implemented correctly here.
2303-2303: Correctly use the local modifier variable.The code now uses the local
modifiervariable instead of directly usingname_modifier, which is necessary for the dynamic module name functionality to work correctly.tests/test_modules_dynamic_no_as/module2/Snakefile (7)
1-1: Good practice for testing module behavior.This line properly tests that configfile statements in modules are ignored during import, which is an important edge case to verify.
4-5: LGTM!This function correctly returns a distinct value (25) that differs from the same function in module1 (15), allowing for verification that different module instances maintain their separate namespaces.
8-12: Well-structured rule with appropriate output definition.The rule correctly uses temporary output files and references the config dictionary, which helps test that module rules can access workflow-level configuration.
15-21: Verify dynamic module rule references work correctly.Good use of rule expansion and chaining. This rule correctly references another rule from the same module using the
rulesnamespace, which is important for testing that module rule references function properly with dynamic module names.
24-30: LGTM!This rule continues the workflow chain and maintains consistency with the module's naming scheme.
33-35: Effective test for rule inheritance within a module.The rule reuse syntax is correct and tests an important feature - that rule inheritance works properly within dynamically named modules.
38-41: LGTM!This aggregation rule completes the workflow chain and ensures all previous rules are executed.
tests/test_modules_dynamic_no_as/module1/Snakefile (7)
1-1: Good practice for testing module behavior.This line properly tests that configfile statements in modules are ignored during import, which is an important edge case to verify.
4-5: LGTM!This function correctly returns a distinct value (15) that differs from the same function in module2 (25), allowing for verification that different module instances maintain their separate namespaces.
8-12: Well-structured rule with appropriate output definition.The rule correctly uses temporary output files and references the config dictionary, which helps test that module rules can access workflow-level configuration.
15-21: Verify dynamic module rule references work correctly.Good use of rule expansion and chaining. This rule correctly references another rule from the same module using the
rulesnamespace, which is important for testing that module rule references function properly with dynamic module names.
24-30: LGTM!This rule continues the workflow chain and maintains consistency with the module's naming scheme.
33-35: Effective test for rule inheritance within a module.The rule reuse syntax is correct and tests an important feature - that rule inheritance works properly within dynamically named modules.
38-41: LGTM!This aggregation rule completes the workflow chain and ensures all previous rules are executed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/snakefiles/modularization.rst (2)
202-204: Clarify the Dynamic Module Loading Explanation
The new explanation concisely introduces dynamic module loading via thename:parameter. Consider expanding on any limitations or best practices when using this feature so that readers are aware of potential constraints (e.g., naming conflicts or the expected format of the module name).
215-219: Fix Minor Spelling Errors in the Developer Note
There are two minor spelling issues in this note:
- "individualy" should be "individually"
- "seperate" should be "separate"
Consider applying the following diff to correct these typos:- If the optional ``as`` keyword is used, the ``use_as`` parameter needs to be specified for each module individualy. + If the optional ``as`` keyword is used, the ``use_as`` parameter needs to be specified for each module individually. - If it is not provided, as in the example, directly, it can be modified in a seperate variable and the variable is used. + If it is not provided, as in the example, directly, it can be modified in a separate variable and the variable is used.🧰 Tools
🪛 GitHub Check: Check for spelling errors
[failure] 216-216:
individualy ==> individually
[failure] 218-218:
seperate ==> separate🪛 GitHub Actions: Codespell
[error] 216-216: codespell: individualy ==> individually
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/snakefiles/modularization.rst(1 hunks)
🧰 Additional context used
🪛 GitHub Check: Check for spelling errors
docs/snakefiles/modularization.rst
[failure] 216-216:
individualy ==> individually
[failure] 218-218:
seperate ==> separate
🪛 GitHub Actions: Codespell
docs/snakefiles/modularization.rst
[error] 216-216: codespell: individualy ==> individually
⏰ Context from checks skipped due to timeout of 90000ms (40)
- GitHub Check: tests (10, windows-latest, py312, bash)
- GitHub Check: tests (10, windows-latest, py311, bash)
- GitHub Check: tests (10, ubuntu-latest, py312, bash)
- GitHub Check: tests (10, ubuntu-latest, py311, bash)
- GitHub Check: tests (9, windows-latest, py312, bash)
- GitHub Check: tests (9, windows-latest, py311, bash)
- GitHub Check: tests (9, ubuntu-latest, py312, bash)
- GitHub Check: tests (9, ubuntu-latest, py311, bash)
- GitHub Check: tests (8, windows-latest, py312, bash)
- GitHub Check: tests (8, windows-latest, py311, bash)
- GitHub Check: tests (8, ubuntu-latest, py312, bash)
- GitHub Check: tests (8, ubuntu-latest, py311, bash)
- GitHub Check: tests (7, windows-latest, py312, bash)
- GitHub Check: tests (7, windows-latest, py311, bash)
- GitHub Check: tests (7, ubuntu-latest, py312, bash)
- GitHub Check: tests (7, ubuntu-latest, py311, bash)
- GitHub Check: tests (6, windows-latest, py312, bash)
- GitHub Check: tests (6, windows-latest, py311, bash)
- GitHub Check: tests (6, ubuntu-latest, py312, bash)
- GitHub Check: tests (6, ubuntu-latest, py311, bash)
- GitHub Check: tests (5, windows-latest, py312, bash)
- GitHub Check: tests (5, windows-latest, py311, bash)
- GitHub Check: tests (5, ubuntu-latest, py312, bash)
- GitHub Check: tests (5, ubuntu-latest, py311, bash)
- GitHub Check: tests (4, windows-latest, py312, bash)
- GitHub Check: tests (4, windows-latest, py311, bash)
- GitHub Check: tests (4, ubuntu-latest, py312, bash)
- GitHub Check: tests (4, ubuntu-latest, py311, bash)
- GitHub Check: tests (3, windows-latest, py312, bash)
- GitHub Check: tests (3, windows-latest, py311, bash)
- GitHub Check: tests (3, ubuntu-latest, py312, bash)
- GitHub Check: tests (3, ubuntu-latest, py311, bash)
- GitHub Check: tests (2, windows-latest, py312, bash)
- GitHub Check: tests (2, windows-latest, py311, bash)
- GitHub Check: tests (2, ubuntu-latest, py312, bash)
- GitHub Check: tests (2, ubuntu-latest, py311, bash)
- GitHub Check: tests (1, windows-latest, py312, bash)
- GitHub Check: tests (1, windows-latest, py311, bash)
- GitHub Check: tests (1, ubuntu-latest, py312, bash)
- GitHub Check: tests (1, ubuntu-latest, py311, bash)
🔇 Additional comments (1)
docs/snakefiles/modularization.rst (1)
205-214: Review of Dynamic Module Loading Code Example
The provided code snippet clearly illustrates how to iterate over module names using a loop and thename:parameter. It effectively demonstrates the dynamic association between module names and their aliases. Ensure that the indentation and formatting in the actual Snakefile remain consistent with other examples to avoid any confusion.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/snakefiles/modularization.rst (2)
202-214: Enhance Dynamic Module Loading Code Snippet ClarityThe added snippet clearly demonstrates how to iterate over module names dynamically and assign them using the
name:parameter. The use of an f-string (i.e.f"{module}/Snakefile") succinctly constructs the snakefile path. Please verify that the DSL syntax—especially in theuse rule * from module as use_as*statement—matches Snakemake’s intended specification. Additionally, consider providing a brief inline comment within the snippet or an accompanying sentence to explain how theuse_asvariable is utilized in the dynamic aliasing process.
215-219: Clarify Note on "as" Keyword UsageThe note effectively points out that when the optional
askeyword is used, a correspondinguse_asparameter must be specified for each module individually and cannot be modified within theuse rulestatement. For improved clarity and consistency, consider rephrasing this note to emphasize that this constraint ensures a one-to-one mapping between module names and their aliases.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/snakefiles/modularization.rst(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (40)
- GitHub Check: tests (10, windows-latest, py312, bash)
- GitHub Check: tests (10, windows-latest, py311, bash)
- GitHub Check: tests (10, ubuntu-latest, py312, bash)
- GitHub Check: tests (10, ubuntu-latest, py311, bash)
- GitHub Check: tests (9, windows-latest, py312, bash)
- GitHub Check: tests (9, windows-latest, py311, bash)
- GitHub Check: tests (9, ubuntu-latest, py312, bash)
- GitHub Check: tests (9, ubuntu-latest, py311, bash)
- GitHub Check: tests (8, windows-latest, py312, bash)
- GitHub Check: tests (8, windows-latest, py311, bash)
- GitHub Check: tests (8, ubuntu-latest, py312, bash)
- GitHub Check: tests (8, ubuntu-latest, py311, bash)
- GitHub Check: tests (7, windows-latest, py312, bash)
- GitHub Check: tests (7, windows-latest, py311, bash)
- GitHub Check: tests (7, ubuntu-latest, py312, bash)
- GitHub Check: tests (7, ubuntu-latest, py311, bash)
- GitHub Check: tests (6, windows-latest, py312, bash)
- GitHub Check: tests (6, windows-latest, py311, bash)
- GitHub Check: tests (6, ubuntu-latest, py312, bash)
- GitHub Check: tests (6, ubuntu-latest, py311, bash)
- GitHub Check: tests (5, windows-latest, py312, bash)
- GitHub Check: tests (5, windows-latest, py311, bash)
- GitHub Check: tests (5, ubuntu-latest, py312, bash)
- GitHub Check: tests (5, ubuntu-latest, py311, bash)
- GitHub Check: tests (4, windows-latest, py312, bash)
- GitHub Check: tests (4, windows-latest, py311, bash)
- GitHub Check: tests (4, ubuntu-latest, py312, bash)
- GitHub Check: tests (4, ubuntu-latest, py311, bash)
- GitHub Check: tests (3, windows-latest, py312, bash)
- GitHub Check: tests (3, windows-latest, py311, bash)
- GitHub Check: tests (3, ubuntu-latest, py312, bash)
- GitHub Check: tests (3, ubuntu-latest, py311, bash)
- GitHub Check: tests (2, windows-latest, py312, bash)
- GitHub Check: tests (2, windows-latest, py311, bash)
- GitHub Check: tests (2, ubuntu-latest, py312, bash)
- GitHub Check: tests (2, ubuntu-latest, py311, bash)
- GitHub Check: tests (1, windows-latest, py312, bash)
- GitHub Check: tests (1, windows-latest, py311, bash)
- GitHub Check: tests (1, ubuntu-latest, py312, bash)
- GitHub Check: tests (1, ubuntu-latest, py311, bash)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
tests/test_modules_semi_dynamic/config/config.yaml (1)
1-1: Add a trailing newline to the YAML file.Best practice for configuration files is to end with a newline character to improve readability and compatibility with Unix text processing tools.
-test: 1 +test: 1 +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: no new line character at the end of file
(new-line-at-end-of-file)
tests/test_modules_dynamic_import_rules/config/config.yaml (1)
1-1: Add a trailing newline to the YAML file.Best practice for configuration files is to end with a newline character to improve readability and compatibility with Unix text processing tools.
-test: 1 +test: 1 +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: no new line character at the end of file
(new-line-at-end-of-file)
tests/test_modules_dynamic_import_rules/Snakefile (1)
25-29: Add trailing comma for consistency in expand lists.The expand call on line 28 is missing a trailing comma. Adding it would maintain consistency with line 27.
input: expand("results/testmodule/b2/{name}.txt", name="test"), - expand("results/testmodule/b2/{name}.tsv", name="test"), + expand("results/testmodule/b2/{name}.tsv", name="test"),Additionally, add a newline at the end of the file to follow best practices.
tests/test_modules_dynamic_import_rules/module2/Snakefile (1)
20-27: Add a trailing newline to the file.Similar to the YAML files, add a newline at the end of the file to follow best practices.
shell: - "cat {input} > {output}" + "cat {input} > {output}" +Also note that rule c correctly references rule b's output using
rules.b.output, which is a good practice.tests/test_modules_semi_dynamic/module1/Snakefile (1)
12-12: Consider documenting the expected config structure.The rule references
{config[test]}which assumes the importing workflow will provide this configuration value. A brief comment about the expected config structure would improve maintainability.tests/test_modules_semi_dynamic/Snakefile (1)
19-23: Consider simplifying the expand expressionsThe nested expand and multiext calls are a bit complex. For clarity, especially in a test file, consider simplifying this expression.
- multiext(expand("results/testmodule/c1/{name}.", name="test")[0], "tsv", "txt"), - multiext(expand("results/testmodule/c2/{name}.", name="test")[0], "tsv", "txt"), + multiext("results/testmodule/c1/test.", "tsv", "txt"), + multiext("results/testmodule/c2/test.", "tsv", "txt"),tests/test_modules_dynamic_import_rules/module3/Snakefile (1)
11-36: Rules b, c, and d follow a repetitive patternRules b, c, and d follow a similar pattern of taking the previous rule's output as input and generating a new file. This is fine for testing purposes, but in production code, you might consider using a more parameterized approach for such similar rules.
For production code (not necessary for this test), you might consider a more concise pattern like:
rule_all = [] extensions = [".out", ".txt", ".tsv"] for i, ext in enumerate(extensions[1:], 1): prev_ext = extensions[i-1] rule_name = f"rule_{chr(97+i)}" # b, c, d rule rule_name: input: expand(f"results/b2/{{name}}{prev_ext}", name="test") output: f"results/b2/{{name}}{ext}" shell: "cat {input} > {output}" rule_all.append(expand(f"results/b2/{{name}}{ext}", name="test"))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
tests/test_modules_semi_dynamic/expected-results/results/testmodule/b1/test.outis excluded by!**/*.outtests/test_modules_semi_dynamic/expected-results/results/testmodule/b2/test.outis excluded by!**/*.out
📒 Files selected for processing (11)
tests/test_modules_dynamic_import_rules/Snakefile(1 hunks)tests/test_modules_dynamic_import_rules/config/config.yaml(1 hunks)tests/test_modules_dynamic_import_rules/expected-results/results/testmodule/b2/test.txt(1 hunks)tests/test_modules_dynamic_import_rules/module1/Snakefile(1 hunks)tests/test_modules_dynamic_import_rules/module2/Snakefile(1 hunks)tests/test_modules_dynamic_import_rules/module3/Snakefile(1 hunks)tests/test_modules_semi_dynamic/Snakefile(1 hunks)tests/test_modules_semi_dynamic/config/config.yaml(1 hunks)tests/test_modules_semi_dynamic/module1/Snakefile(1 hunks)tests/test_modules_semi_dynamic/module2/Snakefile(1 hunks)tests/tests.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/test_modules_dynamic_import_rules/expected-results/results/testmodule/b2/test.txt
🧰 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.
tests/tests.py
🪛 YAMLlint (1.35.1)
tests/test_modules_dynamic_import_rules/config/config.yaml
[error] 1-1: no new line character at the end of file
(new-line-at-end-of-file)
tests/test_modules_semi_dynamic/config/config.yaml
[error] 1-1: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (38)
- GitHub Check: tests (10, windows-latest, py312, bash)
- GitHub Check: tests (10, windows-latest, py311, bash)
- GitHub Check: tests (10, ubuntu-latest, py312, bash)
- GitHub Check: tests (10, ubuntu-latest, py311, bash)
- GitHub Check: tests (9, windows-latest, py312, bash)
- GitHub Check: tests (9, windows-latest, py311, bash)
- GitHub Check: tests (9, ubuntu-latest, py312, bash)
- GitHub Check: tests (9, ubuntu-latest, py311, bash)
- GitHub Check: tests (8, windows-latest, py312, bash)
- GitHub Check: tests (8, windows-latest, py311, bash)
- GitHub Check: tests (8, ubuntu-latest, py312, bash)
- GitHub Check: tests (8, ubuntu-latest, py311, bash)
- GitHub Check: tests (7, windows-latest, py312, bash)
- GitHub Check: tests (7, windows-latest, py311, bash)
- GitHub Check: tests (7, ubuntu-latest, py312, bash)
- GitHub Check: tests (7, ubuntu-latest, py311, bash)
- GitHub Check: tests (6, windows-latest, py312, bash)
- GitHub Check: tests (6, windows-latest, py311, bash)
- GitHub Check: tests (6, ubuntu-latest, py312, bash)
- GitHub Check: tests (6, ubuntu-latest, py311, bash)
- GitHub Check: tests (5, windows-latest, py312, bash)
- GitHub Check: tests (5, windows-latest, py311, bash)
- GitHub Check: tests (5, ubuntu-latest, py312, bash)
- GitHub Check: tests (5, ubuntu-latest, py311, bash)
- GitHub Check: tests (4, windows-latest, py312, bash)
- GitHub Check: tests (4, windows-latest, py311, bash)
- GitHub Check: tests (4, ubuntu-latest, py312, bash)
- GitHub Check: tests (4, ubuntu-latest, py311, bash)
- GitHub Check: tests (3, windows-latest, py312, bash)
- GitHub Check: tests (3, windows-latest, py311, bash)
- GitHub Check: tests (3, ubuntu-latest, py312, bash)
- GitHub Check: tests (3, ubuntu-latest, py311, bash)
- GitHub Check: tests (2, windows-latest, py312, bash)
- GitHub Check: tests (2, windows-latest, py311, bash)
- GitHub Check: tests (2, ubuntu-latest, py312, bash)
- GitHub Check: tests (2, ubuntu-latest, py311, bash)
- GitHub Check: tests (1, windows-latest, py312, bash)
- GitHub Check: tests (1, windows-latest, py311, bash)
🔇 Additional comments (31)
tests/test_modules_dynamic_import_rules/Snakefile (3)
5-14: Well-implemented dynamic module creation!The dynamic module creation using a for loop is a clean approach for handling multiple similar modules. This effectively demonstrates the new dynamic module name feature.
16-18: Clean rule import with output modification.Good use of the rule import syntax with output path modification.
20-22: Appropriate rule import patterns.The multiple rule import and wildcard import with exclusions demonstrate good use of Snakemake's module system features.
tests/test_modules_dynamic_import_rules/module2/Snakefile (3)
1-1: Note about non-existent config file.The comment indicates this configfile statement should be ignored during module import. This is a good test case for checking that the module system correctly handles this situation.
4-9: Well-structured rule with temp directive.Good use of the temp directive for intermediate files and proper reference to the config value.
11-18: Rule input properly references previous rule output.The expand function correctly resolves the input patterns.
tests/test_modules_semi_dynamic/module1/Snakefile (3)
4-5: Good implementation of module function.The
some_funcfunction correctly returns 15, which aligns with the assertion in the main Snakefile that verifiesmodule1.some_func() == 15. This confirms the module's functions are properly accessible after dynamic import.
17-17: Good use of rule references.Using
rules.x.outputinstead of hardcoded paths is a good practice that improves maintainability. If rule outputs change, the dependencies automatically update without requiring changes in multiple places.Also applies to: 26-26, 40-40
33-35: Good demonstration of rule aliasing.The rule alias demonstrates how to create variants of module rules, which is an important feature to test alongside dynamic module naming. This ensures that all module functionality works correctly with the new dynamic naming feature.
tests/test_modules_semi_dynamic/module2/Snakefile (7)
1-1: LGTM on config declarationThe configfile statement is properly documented with a comment explaining it will be ignored during module import.
4-5: LGTM - Function correctly definedThe
some_func()function is properly defined to return the constant value 25, which appears to be tested in the module dynamic tests.
8-12: LGTM - Rule 'a' properly structuredRule 'a' is correctly defined with a temporary output file and uses the config dictionary in the shell command.
15-21: LGTM - Rule 'b' correctly references rule 'a'Rule 'b' properly uses the output from rule 'a' as input with the expand function and fixed wildcard.
24-30: LGTM - Rule 'c_tsv' properly structuredRule 'c_tsv' correctly uses the output from rule 'b' as input and produces the expected output files.
33-35: LGTM - Rule aliasing implemented correctlyThe rule aliasing mechanism is properly implemented, reusing rule 'c_tsv' as 'c_txt' with a different output file extension.
38-40: LGTM - Rule 'all' serves as target ruleRule 'all' properly aggregates the outputs from rule 'c_tsv', which is the standard pattern for defining target rules.
tests/tests.py (7)
1440-1444: LGTM - Test for modules with nameThis test properly verifies that modules with an explicit name parameter work correctly.
1447-1452: LGTM - Test for modules without name correctly expects failureThis test verifies that modules without a name parameter fail as expected with
shouldfail=True.
1455-1460: LGTM - Test for modules with two names correctly expects failureThis test properly verifies that modules with two names fail as expected with
shouldfail=True.
1463-1464: LGTM - Test for dynamic modulesThis test properly verifies the functionality of dynamic modules.
1467-1468: LGTM - Test for semi-dynamic modulesThis test verifies the functionality of semi-dynamic modules, which is relevant to the module2 Snakefile provided in this PR.
1471-1472: LGTM - Test for importing rules from dynamic modulesThis test properly verifies the functionality of importing rules from dynamic modules.
1475-1476: LGTM - Test for dynamic modules without 'as' keywordThis test properly verifies the functionality of dynamic modules without using the 'as' keyword.
tests/test_modules_semi_dynamic/Snakefile (3)
6-14: Module name set dynamically using loop variable - great test of the new feature!This is a good example of how to use the new dynamic module naming feature. The code iterates over a list of module names and associates each with an alias.
16-17: Rule imports using wildcards look goodThe wildcard imports from both modules demonstrate how rules can be properly imported from dynamically named modules.
25-26: Good use of assertions to verify module functionalityThese assertions confirm that the dynamically imported modules work correctly and their functions return the expected values.
tests/test_modules_dynamic_import_rules/module3/Snakefile (2)
1-2: Non-existent config comment is appropriately documentedThe comment explaining that this configfile statement should be ignored during module import is helpful for understanding the test's purpose.
4-9: Rule 'a' properly defined with temporary outputThe rule correctly uses the temp() directive for intermediate files and references the config parameter, which is good for testing config passing to modules.
tests/test_modules_dynamic_import_rules/module1/Snakefile (3)
1-6: Rule 'a' correctly defined with temporary outputThis rule appropriately creates a temporary output file that will be removed once it's no longer needed by downstream rules.
8-15: Rule 'b' properly uses output from rule 'a' as inputThe rule correctly references the output from rule 'a' using the expand function with a fixed name parameter.
17-23: Rule 'c' completes the pipeline with correct input/output pathsThis rule forms the end of the pipeline by taking output from rule 'b' and generating the final .txt file in the c1 directory.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
snakemake/workflow.py (1)
2281-2302: Enhanced module resolution logic for dynamic module names.The code now supports dynamic module name resolution by checking if the module name exists in the current frame's global context when not found in the modules dictionary. This provides a more flexible mechanism for resolving module references.
One improvement to consider:
Consider using Python's recommended exception chaining syntax for clearer error tracing:
- raise WorkflowError( - "Module {} has not been registered with 'module' statement before using it in 'use rule' statement.".format( - from_module - ) - ) + raise WorkflowError( + "Module {} has not been registered with 'module' statement before using it in 'use rule' statement.".format( + from_module + ) + ) from None🧰 Tools
🪛 Ruff (0.8.2)
2298-2302: 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 selected for processing (1)
snakemake/workflow.py(2 hunks)
🧰 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
🪛 Ruff (0.8.2)
snakemake/workflow.py
2298-2302: 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 (2)
snakemake/workflow.py (2)
2249-2258: Parameter change allows for dynamic module names.The
nameparameter is now optional with a default value ofNone. This change enables flexibility in module handling, allowing modules to be used without explicitly specifying names.
2304-2306: Updated parameter passing for module rule usage.The
modifiervariable is now correctly passed instead ofname_modifier, ensuring that any dynamically resolved module names are properly handled.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
docs/snakefiles/modularization.rst (2)
202-205: Documentation Clarity: Dynamic Module Name ExplanationThe added text clearly explains that dynamic loading of modules can be achieved by providing the
namekeyword instead of a fixed module name. This is well aligned with the PR’s objective. Consider emphasizing practical scenarios (e.g., reading module names from a config file) with a brief inline example or remark if space permits.
216-223: Notes on Alias Mapping and Naming RestrictionsThe notes provided in this section are detailed and clarify important constraints:
- Ensuring that when using the
askeyword within a loop, a separate alias variable is used for one-to-one mapping.- Prohibiting the specification of the module name both after the
modulekeyword and thenameparameter.These clarifications are useful. To further enhance readability, consider rephrasing minor parts. For instance, in line 217, change “ensures a one-to-one mapping between module names and their aliases” to “ensures a unique one-to-one mapping between module names and aliases” for added clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/snakefiles/modularization.rst(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: tests (10, windows-latest, py312, bash)
- GitHub Check: tests (10, windows-latest, py311, bash)
- GitHub Check: tests (10, ubuntu-latest, py312, bash)
- GitHub Check: tests (10, ubuntu-latest, py311, bash)
- GitHub Check: tests (4, ubuntu-latest, py312, bash)
- GitHub Check: tests (4, ubuntu-latest, py311, bash)
- GitHub Check: tests (3, windows-latest, py312, bash)
- GitHub Check: tests (3, windows-latest, py311, bash)
- GitHub Check: tests (3, ubuntu-latest, py312, bash)
- GitHub Check: tests (3, ubuntu-latest, py311, bash)
- GitHub Check: tests (2, windows-latest, py312, bash)
- GitHub Check: tests (2, windows-latest, py311, bash)
- GitHub Check: tests (2, ubuntu-latest, py312, bash)
- GitHub Check: tests (2, ubuntu-latest, py311, bash)
- GitHub Check: tests (1, windows-latest, py312, bash)
- GitHub Check: tests (1, windows-latest, py311, bash)
- GitHub Check: tests (1, ubuntu-latest, py312, bash)
- GitHub Check: tests (1, ubuntu-latest, py311, bash)
…make into dynamic_module_name
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/snakemake/workflow.py (1)
2281-2302: Enhanced module lookup and dynamic name handlingThe implementation adds support for retrieving module references from global variables, with proper handling of dynamic name modifiers. This enables:
- Using module variables dynamically in global scope
- Applying dynamic name modifiers based on global variables
- Proper error handling when a module is not found
This is a well-implemented enhancement that aligns with the PR's purpose of supporting dynamic module names.
One minor improvement could be to use the
from errsyntax when re-raising the exception to preserve the traceback, as indicated by the static analysis hint:- raise WorkflowError( - "Module {} has not been registered with 'module' statement before using it in 'use rule' statement.".format( - from_module - ) - ) + raise WorkflowError( + "Module {} has not been registered with 'module' statement before using it in 'use rule' statement.".format( + from_module + ) + ) from None🧰 Tools
🪛 Ruff (0.8.2)
2298-2302: 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 (2)
pyproject.tomlis excluded by!pyproject.tomlsrc/snakemake/report/html_reporter/template/logo.svgis excluded by!**/*.svg
📒 Files selected for processing (14)
.github/workflows/docs.yml(2 hunks).gitignore(1 hunks)MANIFEST.in(1 hunks)setup.py(1 hunks)snakemake/_version.py(0 hunks)src/snakemake/__init__.py(1 hunks)src/snakemake/cli.py(1 hunks)src/snakemake/parser.py(5 hunks)src/snakemake/workflow.py(2 hunks)tests/test_delete_all_output/Snakefile(1 hunks)tests/test_github_issue_3265_respect_dryrun_delete_all/Snakefile(1 hunks)tests/test_github_issue_3265_respect_dryrun_delete_all/Snakefile_inner(1 hunks)tests/tests.py(2 hunks)versioneer.py(0 hunks)
💤 Files with no reviewable changes (2)
- versioneer.py
- snakemake/_version.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.
src/snakemake/__init__.pysrc/snakemake/cli.pysetup.pysrc/snakemake/workflow.pysrc/snakemake/parser.pytests/tests.py
🪛 Ruff (0.8.2)
src/snakemake/__init__.py
8-8: snakemake._version.version imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
src/snakemake/workflow.py
2298-2302: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🪛 actionlint (1.7.4)
.github/workflows/docs.yml
21-21: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
48-48: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms (41)
- GitHub Check: tests (10, windows-latest, py312, bash)
- GitHub Check: tests (10, windows-latest, py311, bash)
- GitHub Check: tests (10, ubuntu-latest, py312, bash)
- GitHub Check: tests (10, ubuntu-latest, py311, bash)
- GitHub Check: tests (9, windows-latest, py312, bash)
- GitHub Check: tests (9, windows-latest, py311, bash)
- GitHub Check: tests (9, ubuntu-latest, py312, bash)
- GitHub Check: tests (9, ubuntu-latest, py311, bash)
- GitHub Check: tests (8, windows-latest, py312, bash)
- GitHub Check: tests (8, windows-latest, py311, bash)
- GitHub Check: tests (8, ubuntu-latest, py312, bash)
- GitHub Check: tests (8, ubuntu-latest, py311, bash)
- GitHub Check: tests (7, windows-latest, py312, bash)
- GitHub Check: tests (7, windows-latest, py311, bash)
- GitHub Check: tests (7, ubuntu-latest, py312, bash)
- GitHub Check: tests (7, ubuntu-latest, py311, bash)
- GitHub Check: tests (6, windows-latest, py312, bash)
- GitHub Check: tests (6, windows-latest, py311, bash)
- GitHub Check: tests (6, ubuntu-latest, py312, bash)
- GitHub Check: tests (6, ubuntu-latest, py311, bash)
- GitHub Check: tests (5, windows-latest, py312, bash)
- GitHub Check: tests (5, windows-latest, py311, bash)
- GitHub Check: tests (5, ubuntu-latest, py312, bash)
- GitHub Check: tests (5, ubuntu-latest, py311, bash)
- GitHub Check: tests (4, windows-latest, py312, bash)
- GitHub Check: tests (4, windows-latest, py311, bash)
- GitHub Check: tests (4, ubuntu-latest, py312, bash)
- GitHub Check: tests (4, ubuntu-latest, py311, bash)
- GitHub Check: tests (3, windows-latest, py312, bash)
- GitHub Check: tests (3, windows-latest, py311, bash)
- GitHub Check: tests (3, ubuntu-latest, py312, bash)
- GitHub Check: tests (3, ubuntu-latest, py311, bash)
- GitHub Check: tests (2, windows-latest, py312, bash)
- GitHub Check: tests (2, windows-latest, py311, bash)
- GitHub Check: tests (2, ubuntu-latest, py312, bash)
- GitHub Check: tests (2, ubuntu-latest, py311, bash)
- GitHub Check: tests (1, windows-latest, py312, bash)
- GitHub Check: apidocs
- GitHub Check: tests (1, windows-latest, py311, bash)
- GitHub Check: tests (1, ubuntu-latest, py312, bash)
- GitHub Check: tests (1, ubuntu-latest, py311, bash)
🔇 Additional comments (25)
src/snakemake/cli.py (1)
2096-2096: Added dryrun parameter to delete_output method callThis is an important improvement that passes the
dryrunflag to thedelete_outputmethod, allowing the function to respect the dry-run mode when deleting outputs. This addresses GitHub issue #3265 mentioned in the PR description, making the behavior consistent with other commands and preventing accidental file deletion when using--dry-runwith--delete-all-output.tests/test_delete_all_output/Snakefile (1)
16-16: Fixed path to match directory structure in testsThe path has been updated from
somedir/finaltosome_dir/final, ensuring consistency with the directory structure used in the test cases.src/snakemake/__init__.py (1)
8-8: Simplified version importThe code now directly imports the version attribute from
_version.pyrather than using a function call and deleting the reference. This simplifies the version handling mechanism.Note: The static analyzer flags this import as unused, but this is a false positive. The import is correctly being used as
__version__which is a standard module attribute in Python packages.🧰 Tools
🪛 Ruff (0.8.2)
8-8:
snakemake._version.versionimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
setup.py (2)
15-15: Updated import path to reflect new directory structureThe path has been modified to include the
srcdirectory, which aligns with the move of the snakemake module into asrcdirectory structure. This is a good practice for Python package organization.
23-23: Switched to SCM-based versioningThe setup now uses
use_scm_version=Trueinstead of the previous versioneer-based approach. This simplifies version management by using git tags directly through setuptools_scm..gitignore (2)
33-34: Path updates to reflect new src directory structureThe changes to .gitignore correctly reflect the project's reorganization, where snakemake code has been moved into a src/ directory structure.
39-39: Include pixi.lock in ignore listProperly ignoring the pixi.lock file prevents accidental commits of this environment lockfile.
src/snakemake/workflow.py (1)
2249-2258: Adding optional module name parameterMaking the
nameparameter optional in themodulemethod is a key change that enables dynamic module names, which aligns with the PR objective.MANIFEST.in (2)
1-7: Updated manifest inclusion rulesThe package manifest has been updated to explicitly include necessary files and use a more restrictive default exclusion policy. This approach ensures better control over what gets included in the package distribution.
8-13: Include src directory and global exclusionsAdding the src directory correctly aligns with the project's structural changes and maintaining the test directory inclusion ensures tests will be available in source distributions. The global exclusions for backup files, compiled Python files, and shared object files follow best practices.
tests/test_github_issue_3265_respect_dryrun_delete_all/Snakefile (1)
1-37: New test for dry-run delete output functionalityThis test file verifies that the --dry-run flag is respected when used with --delete-all-output and --delete-temp-output operations. While this doesn't appear directly related to the dynamic module name feature mentioned in the PR objective, it's a valuable test that ensures these operations don't actually delete files in dry-run mode.
tests/test_github_issue_3265_respect_dryrun_delete_all/Snakefile_inner (1)
1-76: Well-structured test file with comprehensive coverage of output types.This Snakefile effectively tests different output types (regular files, symlinks, protected files, directories, temp files) which is important for verifying proper handling during deletion operations.
tests/tests.py (2)
47-50: Good test coverage for the dryrun delete behavior.This test function properly verifies the fix for GitHub issue #3265 regarding respecting dry-run when deleting all outputs.
1445-1482: Comprehensive test coverage for the dynamic module name feature.These test functions effectively cover various scenarios for module naming:
- Basic module naming
- Error cases (no name, two names)
- Dynamic module usage
- Semi-dynamic modules
- Importing rules from dynamic modules
- Dynamic modules without the 'as' keyword
The tests follow the project's consistent pattern and properly mark expected failures.
src/snakemake/parser.py (6)
870-874: Well-implemented methods for module keyword state handling.These methods properly implement the formatting for module keyword parameters, maintaining consistency with the existing codebase.
877-878: Good addition of ModuleName class.The
ModuleNameclass is appropriately implemented as a subclass ofModuleKeywordState, following the existing pattern for keyword handlers.
927-927: Proper initialization of modulename attribute.The
modulenameattribute is correctly initialized to None in the constructor.
932-938: Robust error handling for module names.The
end()method effectively handles both cases: when a module name is provided or when it's missing. The error message is clear and informative.
949-950: Proper handling of module name assignment.The
name()method correctly assigns the token string as the module name and sets thehas_nameflag.
969-976: Good validation for ambiguous module naming.This block properly checks for ambiguity when a module name is specified both directly after the module keyword and through the name keyword in the module block.
.github/workflows/docs.yml (5)
27-30: Consistent dependency installation commandsThe commands in the "Setup doc dependencies" step now explicitly invoke pip via Python (i.e.
python -m pip install ...). This enhances consistency and clarity compared to the previous usage.
31-35: Verify Sphinx build optionsThe "Build docs" step uses the command
make html SPHINXOPTS="-W --keep-going -n"
which appears to enforce stricter warning handling while building the docs. Please double-check that these options align with your documentation quality objectives.
36-41: Clean formatting for artifact uploadThe "Upload documentation" step includes a minor formatting update (newline addition) that improves readability. The configuration specifying the artifact name and path looks correct.
55-57: Consistent dependency installation in apidocsThe updated commands in the "Setup doc dependencies" step for the apidocs job now use
python -m pip install ..., ensuring consistency with the docs job. This makes the installation process clearer and more robust.
63-68: Artifact path clarity in apidocs uploadThe "Upload documentation" step for apidocs correctly specifies the artifact name and the path (
apidocs/_build/html/). The formatting change at line 67 enhances readability.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/snakemake/workflow.py (1)
2284-2307: 🛠️ Refactor suggestionAdd explanatory comments for dynamic module name mechanism
The implementation for dynamic module names looks solid, but lacks sufficient explanation as requested in a previous review comment. Please add explanatory comments that describe how the dynamic module name resolution works and what kind of fallback it provides.
except KeyError: - # check for dynamic module import - # if from_module was not registered, we check if it is a known - # python variable in the current frame + # Dynamic module name resolution: + # If the static module name is not found in registered modules, + # we check if it's a variable in the calling scope that contains a module name. + # This allows for runtime-determined module names, where the actual module + # name is stored in a variable rather than being hardcoded. from inspect import currentframe if from_module in currentframe().f_back.f_globals: module = self.modules[ currentframe().f_back.f_globals[from_module] ] - # for dynamic module names the name modifier must also be adjusted - # to avoid ambiguous module names + # For dynamic module names, the name modifier must also be adjusted dynamically + # to avoid ambiguous module names. If name_modifier ends with '*', + # use the variable value plus '*', otherwise use the variable value directly. if name_modifier is not None: if name_modifier.endswith("*"): modifier = f"{currentframe().f_back.f_globals[name_modifier[:-1]]}*" else: modifier = currentframe().f_back.f_globals[ name_modifier ] else: - raise WorkflowError( + raise WorkflowError( "Module {} has not been registered with 'module' statement before using it in 'use rule' statement.".format( from_module ) )🧰 Tools
🪛 Ruff (0.8.2)
2303-2307: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
🧹 Nitpick comments (2)
src/snakemake/workflow.py (2)
2303-2307: Improve error handling with exception chainingThe error handling could be improved by using exception chaining with
raise ... from errto preserve the original exception context.else: - raise WorkflowError( + raise WorkflowError( "Module {} has not been registered with 'module' statement before using it in 'use rule' statement.".format( from_module ) - ) + ) from None🧰 Tools
🪛 Ruff (0.8.2)
2303-2307: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
2289-2302: Add error handling for missing dynamic variable valuesThe code assumes that if the variable name exists, it will contain a valid module name string. Consider adding error handling for cases where the variable exists but its value is not a valid module name.
if from_module in currentframe().f_back.f_globals: + module_name = currentframe().f_back.f_globals[from_module] + if module_name not in self.modules: + raise WorkflowError( + f"Dynamic module variable '{from_module}' resolves to '{module_name}', which is not a registered module." + ) module = self.modules[ - currentframe().f_back.f_globals[from_module] + module_name ] # for dynamic module names the name modifier must also be adjusted # to avoid ambiguous module names if name_modifier is not None: if name_modifier.endswith("*"): - modifier = f"{currentframe().f_back.f_globals[name_modifier[:-1]]}*" + name_var = name_modifier[:-1] + if name_var not in currentframe().f_back.f_globals: + raise WorkflowError(f"Dynamic name modifier variable '{name_var}' not found.") + modifier = f"{currentframe().f_back.f_globals[name_var]}*" else: + if name_modifier not in currentframe().f_back.f_globals: + raise WorkflowError(f"Dynamic name modifier variable '{name_modifier}' not found.") modifier = currentframe().f_back.f_globals[ name_modifier ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/snakemake/parser.py(5 hunks)src/snakemake/workflow.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/snakemake/parser.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.
src/snakemake/workflow.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: johanneskoester
PR: snakemake/snakemake#3117
File: tests/test_wrapper/Snakefile:11-11
Timestamp: 2025-03-13T15:41:14.960Z
Learning: Changes made within test cases, such as in `tests/test_wrapper/Snakefile`, are for testing purposes and do not require updates to the project documentation.
🪛 Ruff (0.8.2)
src/snakemake/workflow.py
2303-2307: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
⏰ Context from checks skipped due to timeout of 90000ms (41)
- GitHub Check: tests (10, windows-latest, py312, bash)
- GitHub Check: tests (10, windows-latest, py311, bash)
- GitHub Check: tests (10, ubuntu-latest, py312, bash)
- GitHub Check: tests (10, ubuntu-latest, py311, bash)
- GitHub Check: tests (9, windows-latest, py312, bash)
- GitHub Check: tests (9, windows-latest, py311, bash)
- GitHub Check: tests (9, ubuntu-latest, py312, bash)
- GitHub Check: tests (9, ubuntu-latest, py311, bash)
- GitHub Check: tests (8, windows-latest, py312, bash)
- GitHub Check: tests (8, windows-latest, py311, bash)
- GitHub Check: tests (8, ubuntu-latest, py312, bash)
- GitHub Check: tests (8, ubuntu-latest, py311, bash)
- GitHub Check: tests (7, windows-latest, py312, bash)
- GitHub Check: tests (7, windows-latest, py311, bash)
- GitHub Check: tests (7, ubuntu-latest, py312, bash)
- GitHub Check: tests (7, ubuntu-latest, py311, bash)
- GitHub Check: tests (6, windows-latest, py312, bash)
- GitHub Check: tests (6, windows-latest, py311, bash)
- GitHub Check: tests (6, ubuntu-latest, py312, bash)
- GitHub Check: tests (6, ubuntu-latest, py311, bash)
- GitHub Check: tests (5, windows-latest, py312, bash)
- GitHub Check: tests (5, windows-latest, py311, bash)
- GitHub Check: tests (5, ubuntu-latest, py312, bash)
- GitHub Check: tests (5, ubuntu-latest, py311, bash)
- GitHub Check: tests (4, windows-latest, py312, bash)
- GitHub Check: tests (4, windows-latest, py311, bash)
- GitHub Check: tests (4, ubuntu-latest, py312, bash)
- GitHub Check: tests (4, ubuntu-latest, py311, bash)
- GitHub Check: tests (3, windows-latest, py312, bash)
- GitHub Check: tests (3, windows-latest, py311, bash)
- GitHub Check: tests (3, ubuntu-latest, py312, bash)
- GitHub Check: tests (3, ubuntu-latest, py311, bash)
- GitHub Check: tests (2, windows-latest, py312, bash)
- GitHub Check: tests (2, windows-latest, py311, bash)
- GitHub Check: tests (2, ubuntu-latest, py312, bash)
- GitHub Check: tests (2, ubuntu-latest, py311, bash)
- GitHub Check: tests (1, windows-latest, py312, bash)
- GitHub Check: tests (1, windows-latest, py311, bash)
- GitHub Check: tests (1, ubuntu-latest, py312, bash)
- GitHub Check: tests (1, ubuntu-latest, py311, bash)
- GitHub Check: apidocs
🔇 Additional comments (1)
src/snakemake/workflow.py (1)
2249-2257: Make the module name parameter optionalGood change to make the
nameparameter optional by assigning it a default value ofNone. This allows for dynamic module names as specified in the PR objective.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
tests/test_modules_dynamic_module_as_alias/config/config.yaml (1)
1-1: Add a newline at the end of the file.This YAML file is missing a newline at the end, which is a common best practice for text files and is recommended by YAML linters.
-test: 1 +test: 1 +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: no new line character at the end of file
(new-line-at-end-of-file)
src/snakemake/workflow.py (4)
2284-2322: Add explanatory comment about the purpose of this exception handling.The exception handling block contains a thorough implementation of dynamic module name resolution, but lacks a high-level explanatory comment about its purpose and functionality. Adding a comment would help future developers understand the intent and behavior more quickly.
Based on a previous review comment, please consider adding an explanatory comment at the beginning of this exception handling block:
except KeyError: + # Handle dynamic module names: The module name may refer to a variable in the + # caller's scope that contains the actual module name. This enables using + # variable module names, for example in loops or computed expressions. # Dynamic module name resolution: # If the static module name is not found in the registered modules, # we check if it's a variable in the current scope. from inspect import currentframe🧰 Tools
🪛 Ruff (0.8.2)
2292-2296: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
2311-2315: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
2318-2322: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
2292-2296: Use explicit exception chaining.When raising exceptions within an exception handler, it's a best practice to use explicit exception chaining with
raise ... from errorraise ... from Noneto distinguish them from errors in the exception handling itself.if module_name not in self.modules: raise WorkflowError( "Dynamic module name '{}' resolves to '{}', but has not been registered with a 'module' statement.".format( from_module, module_name ) - ) + ) from None🧰 Tools
🪛 Ruff (0.8.2)
2292-2296: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
2311-2315: Use explicit exception chaining here as well.Similar to the previous suggestion, use explicit exception chaining for clarity.
except KeyError: raise WorkflowError( "Module alias {} not in current frame to resolve dynamic module {} in 'use rule'.".format( name_modifier, module_name ) - ) + ) from None🧰 Tools
🪛 Ruff (0.8.2)
2311-2315: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
2318-2322: Use explicit exception chaining for the final error case.Apply the same pattern for consistent error handling.
else: raise WorkflowError( "Module {} has not been registered with 'module' statement before using it in 'use rule' statement.".format( from_module ) - ) + ) from None🧰 Tools
🪛 Ruff (0.8.2)
2318-2322: 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 (2)
tests/test_modules_dynamic_module_as_alias/expected-results/results/testmodule/b1/test.outis excluded by!**/*.outtests/test_modules_dynamic_module_as_alias/expected-results/results/testmodule/b2/test.outis excluded by!**/*.out
📒 Files selected for processing (6)
src/snakemake/workflow.py(2 hunks)tests/test_modules_dynamic_module_as_alias/Snakefile(1 hunks)tests/test_modules_dynamic_module_as_alias/config/config.yaml(1 hunks)tests/test_modules_dynamic_module_as_alias/module1/Snakefile(1 hunks)tests/test_modules_dynamic_module_as_alias/module2/Snakefile(1 hunks)tests/tests.py(1 hunks)
🧰 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.
tests/tests.pysrc/snakemake/workflow.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: johanneskoester
PR: snakemake/snakemake#3117
File: tests/test_wrapper/Snakefile:11-11
Timestamp: 2025-03-13T15:41:14.960Z
Learning: Changes made within test cases, such as in `tests/test_wrapper/Snakefile`, are for testing purposes and do not require updates to the project documentation.
🪛 Ruff (0.8.2)
src/snakemake/workflow.py
2292-2296: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2311-2315: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2318-2322: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🪛 YAMLlint (1.35.1)
tests/test_modules_dynamic_module_as_alias/config/config.yaml
[error] 1-1: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: tests (10, ubuntu-latest, py312, bash)
- GitHub Check: tests (10, ubuntu-latest, py311, bash)
- GitHub Check: tests (8, windows-latest, py311, bash)
- GitHub Check: tests (3, ubuntu-latest, py312, bash)
🔇 Additional comments (25)
tests/test_modules_dynamic_module_as_alias/Snakefile (3)
6-16: The dynamic module loading implementation looks good.This test case effectively demonstrates how to use dynamic module names with the new feature, iterating over module names and using them as aliases. The implementation is clean and shows both how to specify module parameters and how to import rules from dynamically named modules.
18-22: Well-structured output definition.The
allrule clearly defines the expected outputs usingmultiextandexpandfunctions, making it easy to understand what files are expected from the workflow execution.
24-25: Good validation of module functionality.The assertions validate that the modules are correctly loaded and their functions return the expected values, ensuring the dynamic module loading feature works as intended.
tests/test_modules_dynamic_module_as_alias/module2/Snakefile (4)
1-1: Good explanatory comment for non-existent config.The comment clearly explains that this statement would normally cause an error, but will be ignored during module import, which is helpful for understanding the expected behavior.
4-5: Simple test function implementation.This function provides a straightforward way to verify that the module is correctly loaded in the parent Snakefile through assertions.
8-30: Well-structured rule chain.The rules form a clear processing chain (a → b → c_tsv), each taking the output of the previous rule as input. This is a good pattern for testing the module system as it verifies that rule dependencies work correctly across module boundaries.
33-35: Good demonstration of rule aliasing.Using the
use rule assyntax demonstrates an important feature of Snakemake that should work with dynamic modules. This shows that the feature integrates well with existing functionality.src/snakemake/workflow.py (3)
2249-2258: Good enhancement to make the module name parameter optional.Making the
nameparameter optional with a default value ofNoneis a sensible change that supports dynamic module names while maintaining backward compatibility with existing code.
2281-2282: Clean approach to modifier variable introduction.Creating a local
modifiervariable initialized withname_modifiermakes the code more maintainable, as it allows for dynamic modification of the name without changing the original parameter.
2325-2326: Good use of the dynamically determined modifier variable.Using the dynamically determined
modifierrather than the originalname_modifierparameter ensures that rule names are properly aliased when modules are dynamically loaded, completing the feature implementation.tests/test_modules_dynamic_module_as_alias/module1/Snakefile (7)
1-1: Note about the configfile statementThe comment explains that this statement references a non-existent config file, but will be ignored during module import. This is intentional for testing purposes.
4-5: Function implementation looks goodThis function returns a fixed value (15) that will be used later for assertions in the test workflow.
8-12: Rule 'a' implementation is correctThis rule creates temporary output files and uses shell command to write the config value to the output.
15-21: Rule 'b' properly chains from rule 'a'The rule correctly uses the output from rule 'a' as its input and copies the content to its output file.
24-30: Rule 'c_tsv' correctly builds on rule 'b'This rule properly uses the output from rule 'b' as its input and creates a TSV file.
33-35: Rule alias is well definedThe code correctly uses the 'use rule as' syntax to create an alias of c_tsv with a different output file extension.
38-40: Rule 'all' aggregates outputs correctlyThis rule collects all outputs from rule 'c_tsv', completing the workflow chain.
tests/tests.py (8)
1449-1453: Good implementation of module name testThis test function verifies that modules with explicit names work as expected.
1456-1462: Appropriate negative test for modules without namesThis test correctly expects a failure when a module has no name, using the
shouldfail=Trueparameter.
1464-1470: Good negative test for modules with multiple namesThis test properly verifies that having two module names causes a failure.
1472-1474: Test for dynamic module implementationThis test verifies that dynamic module loading works correctly.
1476-1478: Test for dynamic module with alias functionalityThis test checks that dynamic modules can be used with aliases, which is the main feature being added in this PR.
1480-1482: Test for semi-dynamic module functionalityThis test validates the semi-dynamic module functionality works as expected.
1484-1486: Test for dynamic import of rulesThis test ensures that rules can be dynamically imported from modules.
1488-1490: Test for dynamic modules without the 'as' keywordThis test verifies functionality when dynamic modules are used without the 'as' keyword.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/snakefiles/modularization.rst (1)
207-218: Review of the Dynamic Module Code ExampleThe provided code snippet effectively demonstrates how to dynamically load modules by looping over module names and setting the module's
namevia thenameparameter. The example also correctly shows the corresponding usage with theuse rulestatement and includes a helpful note about not duplicating module name specifications.A couple of minor suggestions:
- Consider adding a brief inline comment (e.g., above the loop) to mention that the
configvariable is expected to be defined in the Snakefile context. This can help readers who may not have full context on whereconfigoriginates.- Verify that the aliasing pattern
as module_name*aligns with the intended dynamic alias mapping and matches examples elsewhere in the documentation.Overall, the example is clear and illustrative of the new dynamic module loading capability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/snakefiles/modularization.rst(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: johanneskoester
PR: snakemake/snakemake#3117
File: tests/test_wrapper/Snakefile:11-11
Timestamp: 2025-03-13T15:41:14.960Z
Learning: Changes made within test cases, such as in `tests/test_wrapper/Snakefile`, are for testing purposes and do not require updates to the project documentation.
⏰ Context from checks skipped due to timeout of 90000ms (41)
- GitHub Check: tests (10, windows-latest, py312, bash)
- GitHub Check: tests (10, windows-latest, py311, bash)
- GitHub Check: tests (10, ubuntu-latest, py312, bash)
- GitHub Check: tests (10, ubuntu-latest, py311, bash)
- GitHub Check: tests (9, windows-latest, py312, bash)
- GitHub Check: tests (9, windows-latest, py311, bash)
- GitHub Check: tests (9, ubuntu-latest, py312, bash)
- GitHub Check: tests (9, ubuntu-latest, py311, bash)
- GitHub Check: tests (8, windows-latest, py312, bash)
- GitHub Check: tests (8, windows-latest, py311, bash)
- GitHub Check: tests (8, ubuntu-latest, py312, bash)
- GitHub Check: tests (8, ubuntu-latest, py311, bash)
- GitHub Check: tests (7, windows-latest, py312, bash)
- GitHub Check: tests (7, windows-latest, py311, bash)
- GitHub Check: tests (7, ubuntu-latest, py312, bash)
- GitHub Check: tests (7, ubuntu-latest, py311, bash)
- GitHub Check: tests (6, windows-latest, py312, bash)
- GitHub Check: tests (6, windows-latest, py311, bash)
- GitHub Check: tests (6, ubuntu-latest, py312, bash)
- GitHub Check: tests (6, ubuntu-latest, py311, bash)
- GitHub Check: tests (5, windows-latest, py312, bash)
- GitHub Check: tests (5, windows-latest, py311, bash)
- GitHub Check: tests (5, ubuntu-latest, py312, bash)
- GitHub Check: tests (5, ubuntu-latest, py311, bash)
- GitHub Check: tests (4, windows-latest, py312, bash)
- GitHub Check: tests (4, windows-latest, py311, bash)
- GitHub Check: tests (4, ubuntu-latest, py312, bash)
- GitHub Check: tests (4, ubuntu-latest, py311, bash)
- GitHub Check: tests (3, windows-latest, py312, bash)
- GitHub Check: tests (3, windows-latest, py311, bash)
- GitHub Check: tests (3, ubuntu-latest, py312, bash)
- GitHub Check: tests (3, ubuntu-latest, py311, bash)
- GitHub Check: tests (2, windows-latest, py312, bash)
- GitHub Check: tests (2, windows-latest, py311, bash)
- GitHub Check: tests (2, ubuntu-latest, py312, bash)
- GitHub Check: tests (2, ubuntu-latest, py311, bash)
- GitHub Check: tests (1, windows-latest, py312, bash)
- GitHub Check: tests (1, windows-latest, py311, bash)
- GitHub Check: tests (1, ubuntu-latest, py312, bash)
- GitHub Check: tests (1, ubuntu-latest, py311, bash)
- GitHub Check: apidocs
🔇 Additional comments (1)
docs/snakefiles/modularization.rst (1)
202-205: Clarify the Dynamic Module Name IntroductionThe added lines clearly introduce the dynamic module naming feature with the new
nameparameter. The explanation is succinct and adequately highlights the shift from fixed module naming to dynamic assignment (e.g., reading from a config file or iterating over modules).
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
docs/snakefiles/modularization.rst (5)
204-205: Dynamic Modules Anchor DefinitionThe anchor label
.. _snakefiles-dynamic-modules:is clearly defined and follows the convention used in the rest of the document. Please ensure that any internal cross-references use this label correctly.
210-213: Dynamic Module Feature ExplanationThe descriptive text for dynamic module loading is concise and informative. It clearly explains that module names are now provided via the
nameparameter rather than directly after themodulekeyword. Consider mentioning explicitly that this resolution happens at runtime for further clarity, although this is optional.
215-223: Dynamic Module Code Example SyntaxThe first code example effectively demonstrates how to iterate over module names dynamically. However, the alias used in the
use rulestatement (as module_name*) might be confusing. It is not immediately clear whether the asterisk is part of the actual syntax or just a placeholder to denote a prefix pattern. A brief clarification or a comment within the snippet could help avoid misinterpretations.
224-227: Enforcement of Name Specification RulesThe note clearly states that it is not allowed to specify the module name both immediately following the
modulekeyword and inside the module definition with thenameparameter. This is an important restriction; consider emphasizing it (for example, using a warning icon or bold text) to ensure users do not overlook this limitation.
234-243: Dynamic Module with Explicit Alias ExampleThe second example using
zipto assign both a module name and an explicit alias is a valuable addition. Similar to the first example, the alias in theuse rulestatement is shown asas alias*, which may be misinterpreted. Clarifying whether the asterisk is syntactic or illustrative would improve the example's clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/snakefiles/modularization.rst(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: johanneskoester
PR: snakemake/snakemake#3117
File: tests/test_wrapper/Snakefile:11-11
Timestamp: 2025-03-14T09:07:33.917Z
Learning: Changes made within test cases, such as in `tests/test_wrapper/Snakefile`, are for testing purposes and do not require updates to the project documentation.
⏰ Context from checks skipped due to timeout of 90000ms (41)
- GitHub Check: tests (10, windows-latest, py312, bash)
- GitHub Check: tests (10, windows-latest, py311, bash)
- GitHub Check: tests (10, ubuntu-latest, py312, bash)
- GitHub Check: tests (10, ubuntu-latest, py311, bash)
- GitHub Check: tests (9, windows-latest, py312, bash)
- GitHub Check: tests (9, windows-latest, py311, bash)
- GitHub Check: tests (9, ubuntu-latest, py312, bash)
- GitHub Check: tests (9, ubuntu-latest, py311, bash)
- GitHub Check: tests (8, windows-latest, py312, bash)
- GitHub Check: tests (8, windows-latest, py311, bash)
- GitHub Check: tests (8, ubuntu-latest, py312, bash)
- GitHub Check: tests (8, ubuntu-latest, py311, bash)
- GitHub Check: tests (7, windows-latest, py312, bash)
- GitHub Check: tests (7, windows-latest, py311, bash)
- GitHub Check: tests (7, ubuntu-latest, py312, bash)
- GitHub Check: tests (7, ubuntu-latest, py311, bash)
- GitHub Check: tests (6, windows-latest, py312, bash)
- GitHub Check: tests (6, windows-latest, py311, bash)
- GitHub Check: tests (6, ubuntu-latest, py312, bash)
- GitHub Check: tests (6, ubuntu-latest, py311, bash)
- GitHub Check: tests (5, windows-latest, py312, bash)
- GitHub Check: tests (5, windows-latest, py311, bash)
- GitHub Check: tests (5, ubuntu-latest, py312, bash)
- GitHub Check: tests (5, ubuntu-latest, py311, bash)
- GitHub Check: tests (4, windows-latest, py312, bash)
- GitHub Check: tests (4, windows-latest, py311, bash)
- GitHub Check: tests (4, ubuntu-latest, py312, bash)
- GitHub Check: tests (4, ubuntu-latest, py311, bash)
- GitHub Check: tests (3, windows-latest, py312, bash)
- GitHub Check: tests (3, windows-latest, py311, bash)
- GitHub Check: tests (3, ubuntu-latest, py312, bash)
- GitHub Check: tests (3, ubuntu-latest, py311, bash)
- GitHub Check: tests (2, windows-latest, py312, bash)
- GitHub Check: tests (2, windows-latest, py311, bash)
- GitHub Check: tests (2, ubuntu-latest, py312, bash)
- GitHub Check: tests (2, ubuntu-latest, py311, bash)
- GitHub Check: tests (1, windows-latest, py312, bash)
- GitHub Check: tests (1, windows-latest, py311, bash)
- GitHub Check: tests (1, ubuntu-latest, py312, bash)
- GitHub Check: apidocs
- GitHub Check: tests (1, ubuntu-latest, py311, bash)
🔇 Additional comments (3)
docs/snakefiles/modularization.rst (3)
206-209: Section Header ConsistencyThe "Dynamic Modules" section header (with the surrounding underlines) is formatted consistently with earlier sections, ensuring clarity and easy navigation for readers.
228-232: Dynamic Resolution Explanation ClarityThe explanation following the note clarifies how dynamic resolution of module names works, particularly when the module name is not registered. The guidance on alias usage and variable scope is helpful and clear. No changes are required here.
233-233: Aliasing Restriction ReminderThe statement that modifying the alias name directly in the
use rulestatement is not allowed for dynamic modules is clear and serves as a useful reminder.
🤖 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>
Allow for dynamic module names resolves snakemake#1923 ### 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 - **New Features** - Enhanced modular workflows enable dynamic and semi-dynamic module configuration with flexible aliasing and output management. - **Documentation** - Updated guidelines provide clear instructions on configuring dynamic module imports and rule aliasing, including a new section on "Dynamic Modules." - **Tests** - Expanded test coverage validates diverse module configurations and expected outcomes, ensuring robust and reliable workflows. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Jens Zentgraf <zentgraf@cs.uni-saarland.de> Co-authored-by: Johanna <johanna_schmitz@posteo.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>
Allow for dynamic module names
resolves #1923
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