WIP fix: cannot unpack non-iterable NoneType object from #3365#3496
WIP fix: cannot unpack non-iterable NoneType object from #3365#3496Hocnonsense wants to merge 4 commits intomainfrom
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
You can disable this status message by setting the 📝 WalkthroughWalkthroughThis pull request introduces a new Snakefile in the Changes
Sequence Diagram(s)sequenceDiagram
participant T as Test Runner (tests/tests.py)
participant S as Snakefile Rule "one"
participant C as Shell Command
participant O as Output File (one.txt)
T->>S: Trigger rule execution (via test_issue3495)
S->>C: Execute shell command (echo parameters)
C->>O: Write output ("one.txt") with result (e.g., "goor job")
T->>T: Validate output against expected result ("c1")
Suggested reviewers
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tests/test_issue3495/Snakefile(1 hunks)tests/test_issue3495/expected-results/one.txt(1 hunks)tests/tests.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.py`: Do not try to improve formatting. Do not suggest type annotations for functions that are defined inside of functions or methods. Do not suggest type annotation of the `s...
**/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of theselfargument of methods.
Do not suggest type annotation of theclsargument of classmethods.
Do not suggest return type annotation if a function or method does not contain areturnstatement.
tests/tests.py
🧬 Code Definitions (1)
tests/tests.py (2)
tests/common.py (2)
run(151-494)dpath(32-35)src/snakemake/workflow.py (1)
run(2264-2265)
⏰ Context from checks skipped due to timeout of 90000ms (35)
- GitHub Check: tests (10, windows-latest, py312)
- GitHub Check: tests (10, ubuntu-latest, py312)
- GitHub Check: tests (10, ubuntu-latest, py311)
- GitHub Check: tests (9, windows-latest, py312)
- GitHub Check: tests (9, ubuntu-latest, py312)
- GitHub Check: tests (9, ubuntu-latest, py311)
- GitHub Check: tests (8, windows-latest, py312)
- GitHub Check: tests (8, ubuntu-latest, py312)
- GitHub Check: tests (8, ubuntu-latest, py311)
- GitHub Check: tests (7, windows-latest, py312)
- GitHub Check: tests (7, ubuntu-latest, py312)
- GitHub Check: tests (7, ubuntu-latest, py311)
- GitHub Check: tests (6, windows-latest, py312)
- GitHub Check: tests (6, ubuntu-latest, py312)
- GitHub Check: tests (6, ubuntu-latest, py311)
- GitHub Check: tests (5, windows-latest, py312)
- GitHub Check: tests (5, ubuntu-latest, py312)
- GitHub Check: tests (5, ubuntu-latest, py311)
- GitHub Check: tests (4, macos-latest, py312)
- GitHub Check: tests (4, windows-latest, py312)
- GitHub Check: tests (4, ubuntu-latest, py312)
- GitHub Check: tests (4, ubuntu-latest, py311)
- GitHub Check: tests (3, macos-latest, py312)
- GitHub Check: tests (3, windows-latest, py312)
- GitHub Check: tests (3, ubuntu-latest, py312)
- GitHub Check: tests (3, ubuntu-latest, py311)
- GitHub Check: tests (2, macos-latest, py312)
- GitHub Check: tests (2, windows-latest, py312)
- GitHub Check: tests (2, ubuntu-latest, py312)
- GitHub Check: tests (2, ubuntu-latest, py311)
- GitHub Check: tests (1, macos-latest, py312)
- GitHub Check: tests (1, windows-latest, py312)
- GitHub Check: tests (1, ubuntu-latest, py312)
- GitHub Check: tests (1, ubuntu-latest, py311)
- GitHub Check: apidocs
🔇 Additional comments (4)
tests/test_issue3495/expected-results/one.txt (1)
1-2: Output file looks correct but simple.This expected output file contains a single line with "c1", which will be used to validate the test case output.
tests/test_issue3495/Snakefile (2)
9-12: Verify the parameter override is working as expected.The rule
oneis used astwowith a parameteroutput="goor job". Since the original rule uses{params}in the shell command without specifying which parameter to access, it's unclear how this parameter will be used. Is the intention that{params}should evaluate to "goor job" when used in this context?
1-7:Details
❓ Verification inconclusive
Rule definition has potential issue with params usage.
The shell command references
{params}but there's noparams:section defined in the rule. This could be intentional if you're testing how the Snakemake workflow handles this situation (perhaps related to the "cannot unpack non-iterable NoneType object" issue mentioned in the PR title).
🏁 Script executed:
#!/bin/bash # Check if the Snakefile works as expected cd tests/test_issue3495 snakemake -nLength of output: 140
Action Required: Verify {params} Usage in the Snakefile Rule
In the rule defined in
tests/test_issue3495/Snakefile(lines 1-7), the shell command references{params}even though noparams:section is provided. This may be intentional—possibly as part of testing for the "cannot unpack non-iterable NoneType object" error—but it's unclear from the current evidence.Our initial test execution did not provide useful feedback because the Snakemake command was not found in the environment:
+ cd tests/test_issue3495 + snakemake -n /.coderabbit_commands_59794b0a-6e19-47f9-b84f-c45dfa2110ac.sh: line 4: snakemake: command not foundNext Steps:
- Manual Verification: Please verify the behavior with Snakemake installed to determine if the missing
params:section results in a runtime error or if it is working as intended.- Rule Adjustment: If the behavior is unexpected, consider either adding a
params:section or updating the shell command to remove the unused{params}reference.tests/tests.py (1)
2249-2252: Test function looks appropriate for issue #3495.The new test function follows the same pattern as other test functions in this file. It's decorated with
@skip_on_windowsand callsrun()with the path to the test directory.
| output="goor job", | ||
|
|
||
|
|
||
| print(UseRuleWith.INHERIT) |
There was a problem hiding this comment.
Missing import for UseRuleWith.
The code references UseRuleWith.INHERIT but there's no import statement for this symbol. Make sure it's properly imported from the appropriate module.
+from snakemake.common.rules import UseRuleWith
rule one:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| print(UseRuleWith.INHERIT) | |
| from snakemake.common.rules import UseRuleWith | |
| print(UseRuleWith.INHERIT) | |
| rule one: | |
| # Rest of the rule goes here |
### Description Follow-up change on merged PR #3624 , inspired by suggested PR #3496 (probs to @Hocnonsense ). ### QC * [x] The PR contains a test case for the changes or the changes are already covered by an existing test case. * [x] The documentation (`docs/`) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved handling of rule parameters to prevent errors when parameters are missing. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
### Description Follow-up change on merged PR snakemake#3624 , inspired by suggested PR snakemake#3496 (probs to @Hocnonsense ). ### QC * [x] The PR contains a test case for the changes or the changes are already covered by an existing test case. * [x] The documentation (`docs/`) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved handling of rule parameters to prevent errors when parameters are missing. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 participants
This is the testcase of #3495
QC
docs/) havn't updated to reflect the changes.Summary by CodeRabbit
New Features
Tests