Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 36 minutes and 25 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUpdated two functions in the snakemake module: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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: 3
🧹 Nitpick comments (1)
snakemake_wrapper_utils/snakemake.py (1)
53-55: Misplaced docstring.Line 55 is not actually a docstring — it follows the
from pathlib import Pathimport on line 53, so Python treats it as a plain string expression statement. Tools likehelp(get_format), Sphinx, and IDEs will not pick it up. Move the string to be the first statement of the function (or convertfrom pathlib import Pathto a module-level import).♻️ Proposed fix
def get_format(path, ignore_compression=True): - from pathlib import Path - """Get file format from extension, ignoring common compressions on user request""" + from pathlib import Path + if not path: raise ValueError("Path cannot be empty")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@snakemake_wrapper_utils/snakemake.py` around lines 53 - 55, The triple-quoted string after the import is a misplaced docstring and should be moved so tools can see it: either relocate that string as the first statement inside the get_format function (making it the function docstring) or convert the `from pathlib import Path` line into a module-level import and place the descriptive string at top-of-file as the module docstring; update the get_format function signature/location if necessary to ensure help(get_format) and Sphinx/IDEs pick up the docstring.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@snakemake_wrapper_utils/snakemake.py`:
- Line 58: The code lowercases suffixes via "exts = [s.lower() for s in
Path(path).suffixes]" but the compression mapping contains an uppercase key
'".Z": "zlib"', making ".Z" unreachable; update the mapping in the same scope to
use lowercase keys (e.g. replace '".Z"' with '".z"') or add both lowercase and
uppercase variants for the key so the lookup matches the lowercased suffixes,
and while editing verify the intended format label for the classic ".Z"
compression entry (function/variable around the suffixes/mapping logic).
- Around line 83-84: The dictionary compression_fmt contains misspelled entries
for deflate; update it so the canonical extension ".deflate" maps to the value
"deflate" and remove the incorrect keys ".deflat" and ".defalte" (and the
misspelled value "defalte") so callers of compression_fmt receive the correct
"deflate" string; locate the compression_fmt dict in snakemake.py and replace
the two buggy entries with a single correct mapping ".deflate": "deflate".
- Around line 111-113: The branch handling ignore_compression=False currently
maps only the last suffix (exts[-1]) through compression_fmt then bioinfo_fmt,
losing the underlying base format; change the logic to extract both the
compression suffix and the base suffix (e.g., base_ext = exts[-2] if len(exts) >
1 else exts[-1] and comp_ext = exts[-1] when they differ), map base_ext through
bioinfo_fmt and comp_ext through compression_fmt, and return both pieces (e.g.,
a tuple (base_fmt, compression_fmt_or_None) or a small dataclass) instead of a
single string; update any callers of this function to accept the new return
shape.
---
Nitpick comments:
In `@snakemake_wrapper_utils/snakemake.py`:
- Around line 53-55: The triple-quoted string after the import is a misplaced
docstring and should be moved so tools can see it: either relocate that string
as the first statement inside the get_format function (making it the function
docstring) or convert the `from pathlib import Path` line into a module-level
import and place the descriptive string at top-of-file as the module docstring;
update the get_format function signature/location if necessary to ensure
help(get_format) and Sphinx/IDEs pick up the docstring.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f4adef37-3c71-4866-a7f9-264c956b801e
📒 Files selected for processing (1)
snakemake_wrapper_utils/snakemake.py
Co-authored-by: Filipe G. Vieira <1151762+fgvieira@users.noreply.github.com>
…emake-wrapper-utils into compression_formats
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
snakemake_wrapper_utils/snakemake.py (1)
100-100: Nit: drop the redundant.keys().
inon a dict already checks keys;.keys()builds an intermediate view unnecessarily.♻️ Proposed refactor
- if ignore_compression and exts[-1] in compression_fmt.keys(): + if ignore_compression and exts[-1] in compression_fmt:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@snakemake_wrapper_utils/snakemake.py` at line 100, The condition uses an unnecessary .keys() call: replace the membership test exts[-1] in compression_fmt.keys() with exts[-1] in compression_fmt to avoid constructing the intermediate keys view; locate the expression in snakemake_wrapper_utils/snakemake.py (the if statement that checks ignore_compression and compression_fmt) and update it accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@snakemake_wrapper_utils/snakemake.py`:
- Line 110: The return currently uses ext.rstrip(".") which removes trailing
dots and leaves a leading dot on unknown extensions (ext variable) when neither
compression_fmt nor bioinfo_fmt map it; change the logic to strip a leading dot
instead (use lstrip on ext) so unmapped extensions like ".xyz" become "xyz"
before returning—update the return in the function that looks up compression_fmt
and bioinfo_fmt to return ext.lstrip(".") where ext is the fallback extension.
- Around line 53-55: The triple-quoted string after the import is not a function
docstring; move that documentation so it is the first statement inside the
get_format function (or make it a module-level docstring at the top of the file)
so get_format.__doc__ is populated; specifically, place the existing string
immediately after the def get_format(...) line (before any code or imports
inside the function) and remove the dead string literal after from pathlib
import Path.
---
Nitpick comments:
In `@snakemake_wrapper_utils/snakemake.py`:
- Line 100: The condition uses an unnecessary .keys() call: replace the
membership test exts[-1] in compression_fmt.keys() with exts[-1] in
compression_fmt to avoid constructing the intermediate keys view; locate the
expression in snakemake_wrapper_utils/snakemake.py (the if statement that checks
ignore_compression and compression_fmt) and update it accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 328ca7ca-5500-4795-b374-06532bbd1154
📒 Files selected for processing (1)
snakemake_wrapper_utils/snakemake.py
2 participants
This is being related to PR on snakemake-wrappers.
This PR adds new formats detection, allows to return compression format without breaking retro-compatibility.
New formats can easily be added in the future with addition in the dictionaries.
Summary by CodeRabbit
Bug Fixes
New Features