fixes for #209 #129 #193 #267#270
Merged
mbhall88 merged 3 commits intosnakemake:masterfrom Feb 9, 2026
Merged
Conversation
📝 WalkthroughWalkthroughUpdates to parsing and formatting to support Snakemake v8/v9 features: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🤖 Fix all issues with AI agents
In `@tests/test_formatter.py`:
- Around line 955-968: In the test_shell_interspersed_comments test, remove the
unnecessary inline noqa directive on the comment line that currently reads "#
Merge overlapping coordinates into individual expanded records # noqa: E501"
(inside the snakecode string); update the string in
test_shell_interspersed_comments so the long comment no longer contains " #
noqa: E501" and ensure the test still asserts formatter.get_formatted() ==
snakecode.
Comment on lines
+955
to
+968
| def test_shell_interspersed_comments(self): | ||
| """https://github.com/snakemake/snakefmt/issues/193""" | ||
| snakecode = ( | ||
| "rule:\n" | ||
| f"{TAB * 1}shell:\n" | ||
| f"{TAB * 2}# Concatenate all the files together\n" | ||
| f'{TAB * 2}"cat {{input:q}} 2> {{log.cat:q}} | "\n' | ||
| f"{TAB * 2}# Coordinate sort everything\n" | ||
| f'{TAB * 2}+"sortBed 2> {{log.sort:q}} | "\n' | ||
| f"{TAB * 2}# Merge overlapping coordinates into individual expanded records\n" # noqa: E501 | ||
| f'{TAB * 2}+"mergeBed 2> {{log.merge:q}} 1> {{output:q}}"\n' | ||
| ) | ||
| formatter = setup_formatter(snakecode) | ||
| assert formatter.get_formatted() == snakecode |
There was a problem hiding this comment.
Remove unused noqa directive on line 964.
Static analysis indicates the # noqa: E501 comment is unnecessary since the line doesn't actually trigger an E501 violation.
Suggested fix
- f"{TAB * 2}# Merge overlapping coordinates into individual expanded records\n" # noqa: E501
+ f"{TAB * 2}# Merge overlapping coordinates into individual expanded records\n"📝 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.
Suggested change
| def test_shell_interspersed_comments(self): | |
| """https://github.com/snakemake/snakefmt/issues/193""" | |
| snakecode = ( | |
| "rule:\n" | |
| f"{TAB * 1}shell:\n" | |
| f"{TAB * 2}# Concatenate all the files together\n" | |
| f'{TAB * 2}"cat {{input:q}} 2> {{log.cat:q}} | "\n' | |
| f"{TAB * 2}# Coordinate sort everything\n" | |
| f'{TAB * 2}+"sortBed 2> {{log.sort:q}} | "\n' | |
| f"{TAB * 2}# Merge overlapping coordinates into individual expanded records\n" # noqa: E501 | |
| f'{TAB * 2}+"mergeBed 2> {{log.merge:q}} 1> {{output:q}}"\n' | |
| ) | |
| formatter = setup_formatter(snakecode) | |
| assert formatter.get_formatted() == snakecode | |
| def test_shell_interspersed_comments(self): | |
| """https://github.com/snakemake/snakefmt/issues/193""" | |
| snakecode = ( | |
| "rule:\n" | |
| f"{TAB * 1}shell:\n" | |
| f"{TAB * 2}# Concatenate all the files together\n" | |
| f'{TAB * 2}"cat {{input:q}} 2> {{log.cat:q}} | "\n' | |
| f"{TAB * 2}# Coordinate sort everything\n" | |
| f'{TAB * 2}+"sortBed 2> {{log.sort:q}} | "\n' | |
| f"{TAB * 2}# Merge overlapping coordinates into individual expanded records\n" | |
| f'{TAB * 2}+"mergeBed 2> {{log.merge:q}} 1> {{output:q}}"\n' | |
| ) | |
| formatter = setup_formatter(snakecode) | |
| assert formatter.get_formatted() == snakecode |
🧰 Tools
🪛 Ruff (0.14.14)
[warning] 964-964: Unused noqa directive (non-enabled: E501)
Remove unused noqa directive
(RUF100)
🤖 Prompt for AI Agents
In `@tests/test_formatter.py` around lines 955 - 968, In the
test_shell_interspersed_comments test, remove the unnecessary inline noqa
directive on the comment line that currently reads "# Merge overlapping
coordinates into individual expanded records # noqa: E501" (inside the
snakecode string); update the string in test_shell_interspersed_comments so the
long comment no longer contains " # noqa: E501" and ensure the test still
asserts formatter.get_formatted() == snakecode.
Member
Author
|
PR title check is failing but that doesn't matter as I will do a rebase merge of the PR, so the title won't end up as a commit. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #209
We had implicitly resolved this issue when we updated our snakemake supported dev version
I didn't do these tests the same way as we always necessarily do them (i.e. with the expected formatted code) because we are just testing this syntax doesn't raise an issue.
also closes #129 and closes #193 and closes #267
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.