Update Resistify #51675
Update Resistify #51675
Conversation
📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces several modifications to the Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
🧹 Outside diff range and nitpick comments (2)
recipes/resistify/meta.yaml (2)
50-58: Fix trailing whitespace issues.The description and metadata additions look good, but there are trailing spaces on lines 35, 36, and 58 that should be removed.
- - pytorch - - torchvision + - pytorch + - torchvision ... - +🧰 Tools
🪛 yamllint
[error] 58-58: trailing spaces
(trailing-spaces)
Based on the search results, I can now generate the final response since I have sufficient information about PyTorch and ML framework dependencies in the codebase.
The noarch: python setting should be removed due to platform-specific dependencies
The package includes PyTorch and other ML frameworks that have platform-specific optimizations and dependencies:
- The package directly depends on PyTorch, torchvision, and torchaudio which have platform-specific builds
- The package also depends on
cpuonlywhich is inherently platform-specific- Other packages in the codebase with similar PyTorch dependencies (like vamb, ggcaller, etc.) are not using noarch: python
The noarch: python setting is not appropriate when packages have platform-specific dependencies, as it assumes the package is platform-independent. This should be changed to ensure proper platform-specific builds are created.
🔗 Analysis chain
Line range hint
14-21: Verify if noarch: python is still appropriate.Given the addition of PyTorch and other ML frameworks as dependencies, please verify that the package remains platform-independent. Some ML operations might have platform-specific optimizations or requirements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there are any platform-specific files or build requirements rg -l "platform|architecture|cuda|gpu"Length of output: 26985
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/resistify/meta.yaml (3 hunks)
🧰 Additional context used
🪛 yamllint
recipes/resistify/meta.yaml
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 58-58: trailing spaces
(trailing-spaces)
🔇 Additional comments (3)
recipes/resistify/meta.yaml (3)
2-3: LGTM: Version update and checksum look correct.The version bump to 0.5.0 and its corresponding SHA256 checksum are properly configured.
10-11: LGTM: Source URL change is appropriate.The switch to GitHub source with versioned tags is a good practice, especially given the PyPI file size limitations mentioned in the PR objectives.
34-39: Review ML framework version constraints.The PyTorch ecosystem components (pytorch, torchvision, torchaudio) should have consistent version constraints. Additionally, verify compatibility between:
- PyTorch and transformers 4.31.0
- fair-esm and its PyTorch requirements
#!/bin/bash # Check for any existing version constraints or compatibility requirements rg -A 5 "torch|transformers|fair-esm"🧰 Tools
🪛 yamllint
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
recipes/resistify/meta.yaml
Outdated
| @@ -21,16 +22,22 @@ build: | |||
| requirements: | |||
| host: | |||
| - pip | |||
| - python >=3.9 | |||
| - python | |||
There was a problem hiding this comment.
Consider maintaining Python version constraint.
Removing the Python >=3.9 constraint might lead to compatibility issues, especially with the ML frameworks added as dependencies. Consider keeping version constraints to ensure compatibility.
- python
+ python >=3.9Also applies to: 28-28
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
recipes/resistify/meta.yaml (1)
50-58: Fix trailing space in description section.The metadata additions (doc_url, license_file, and description) are great improvements. However, there's a trailing space issue to fix:
genes in plant genomes. - +🧰 Tools
🪛 yamllint
[error] 58-58: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/resistify/meta.yaml (3 hunks)
🧰 Additional context used
🪛 yamllint
recipes/resistify/meta.yaml
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 58-58: trailing spaces
(trailing-spaces)
🔇 Additional comments (5)
recipes/resistify/meta.yaml (5)
2-3: LGTM: Version update and checksum look correct.The version update to 0.5.0 and the corresponding SHA256 checksum are properly configured.
Line range hint
21-22: LGTM: Appropriate pinning strategy.The
run_exportsconfiguration withmax_pin="x.x"is well-suited for managing potential API/ABI changes.🧰 Tools
🪛 yamllint
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
25-25: Previous Python version constraint comment still applies.The concern about removing Python version constraints raised in the previous review remains valid.
Also applies to: 28-28
34-40:⚠️ Potential issueConsider version constraints for ML dependencies.
The ML dependencies (pytorch, torchvision, torchaudio) should have version constraints to ensure compatibility, especially since transformers is pinned to 4.31.0.
#!/bin/bash # Check if there are any version constraints in other recipes using these dependencies rg -l 'pytorch.*[0-9]' recipes/ rg -l 'transformers.*[0-9]' recipes/Also, fix the trailing spaces in the YAML file:
- pytorch - torchvision - torchaudio + pytorch + torchvision + torchaudio🧰 Tools
🪛 yamllint
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
10-11: Verify GitHub source URL accessibility.The switch from PyPI to GitHub source is correctly configured, but let's verify the URL accessibility.
✅ Verification successful
The previous script execution seems to have been overridden with a different search. Let me try to verify the GitHub URL accessibility again.
GitHub source URL is properly configured and accessible
The GitHub repository and release URL are both accessible:
- Repository exists at github.com/SwiftSeal/resistify (HTTP 200)
- Release archive URL properly redirects to codeload.github.com (HTTP 302)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the GitHub release tag exists and is accessible gh release view v0.5.0 --repo SwiftSeal/resistifyLength of output: 1053
Script:
#!/bin/bash # Verify that the GitHub release tag exists and is accessible curl -I "https://github.com/SwiftSeal/resistify/archive/refs/tags/v0.5.0.tar.gz" # Also check the repository itself curl -I "https://github.com/SwiftSeal/resistify"Length of output: 9186
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
recipes/resistify/meta.yaml (1)
29-31: Consider relaxing strict version pins.The strict version pins (scikit-learn==0.24.2, transformers==4.31.0) might be unnecessarily restrictive. Consider using compatible release specifiers (e.g.,
>=0.24.2,<0.25.0) unless these specific versions are absolutely required.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/resistify/meta.yaml (3 hunks)
🧰 Additional context used
🪛 yamllint
recipes/resistify/meta.yaml
[error] 37-37: trailing spaces
(trailing-spaces)
[error] 38-38: trailing spaces
(trailing-spaces)
[error] 59-59: trailing spaces
(trailing-spaces)
🔇 Additional comments (5)
recipes/resistify/meta.yaml (5)
2-3: LGTM: Version and checksum update.The version bump and new SHA256 checksum are properly configured.
10-11: LGTM: Source URL update.The switch to GitHub source is appropriate given the PyPI file size limitations mentioned in the PR objectives.
Line range hint
20-21: LGTM: Proper run_exports configuration.The pin_subpackage configuration will help manage API/ABI compatibility for downstream dependencies.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
51-58: LGTM: Improved metadata documentation.The addition of documentation URL, license file, and detailed description improves package metadata quality.
36-41: Verify ML framework version compatibility.The addition of PyTorch ecosystem (pytorch, torchvision, torchaudio) along with transformers and fair-esm requires careful version alignment. Please verify:
- The compatibility between PyTorch and transformers 4.31.0
- The minimum required CUDA version if GPU support is needed
✅ Verification successful
Let me gather more information about transformers version compatibility.
PyTorch and transformers compatibility looks acceptable
The transformers version 4.31.0 is compatible with recent PyTorch versions, and since no specific PyTorch version is pinned in the recipe, conda will resolve to a compatible version. The absence of explicit CUDA requirements suggests CPU-only usage, which simplifies the dependency resolution.
Key observations:
- Other recipes in the repository use various PyTorch versions (1.2.0 to 1.13.1) for different purposes
- Not pinning PyTorch version here provides flexibility while ensuring compatibility through conda's dependency resolver
- No CUDA requirements specified, indicating CPU-only usage
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there are any version constraints for PyTorch in the repository rg -A 2 'pytorch.*==' rg -A 2 'torch.*=='Length of output: 1739
Script:
#!/bin/bash # Check if there are any version constraints for transformers and related packages rg -A 2 'transformers.*==' rg -A 2 'fair-esm.*==' # Check if there are any CUDA requirements specified rg -A 2 'cuda' recipes/resistify/Length of output: 220
🧰 Tools
🪛 yamllint
[error] 37-37: trailing spaces
(trailing-spaces)
[error] 38-38: trailing spaces
(trailing-spaces)
recipes/resistify/meta.yaml
Outdated
| - torchvision | ||
| - torchaudio |
There was a problem hiding this comment.
Fix trailing whitespace.
Remove trailing spaces from the following lines:
- Line 37:
pytorch - Line 38:
torchvision - Line 59: Empty line with spaces
Also applies to: 59-59
🧰 Tools
🪛 yamllint
[error] 37-37: trailing spaces
(trailing-spaces)
[error] 38-38: trailing spaces
(trailing-spaces)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/resistify/meta.yaml (3 hunks)
🧰 Additional context used
🪛 yamllint
recipes/resistify/meta.yaml
[error] 59-59: trailing spaces
(trailing-spaces)
🔇 Additional comments (6)
recipes/resistify/meta.yaml (6)
2-3: LGTM: Version update and checksum look correct.The version bump and SHA256 checksum are properly formatted.
Line range hint
20-21: LGTM: Proper version pinning with run_exports.The addition of
run_exportswith appropriate version pinning helps manage API/ABI compatibility, which is a recommended practice in Bioconda.🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
51-58: LGTM: Complete metadata information.The added documentation, license information, and description provide clear and complete metadata for the package.
36-41: Verify ML framework compatibility.The addition of PyTorch ecosystem and transformer libraries significantly increases the dependency footprint. Please ensure:
- All version combinations are tested and compatible
- The package size remains manageable for conda distribution
#!/bin/bash # Check other bioconda packages for similar dependency patterns rg "pytorch.*torchvision.*transformers" -A 5 -B 5
10-11: Verify GitHub release availability.The switch from PyPI to GitHub source is appropriate for handling large package files.
#!/bin/bash # Verify that the GitHub release exists and is accessible gh release view "v0.5.0" --repo "SwiftSeal/resistify" || echo "Release not found"
25-25: Consider relaxing numpy version constraint.While specific version pins for scikit-learn and Python are justified due to ML framework compatibility, the numpy version constraint
<1.23.0seems overly restrictive and might cause dependency conflicts with other packages.Also applies to: 29-31
| Resistify is a nucleotide-binding leucine rich repeat (NLR) annotation | ||
| program, for the purpose of identifying putative disease resistance | ||
| genes in plant genomes. | ||
|
|
There was a problem hiding this comment.
Fix trailing whitespace.
Remove the trailing whitespace on line 59.
-
+📝 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.
🧰 Tools
🪛 yamllint
[error] 59-59: trailing spaces
(trailing-spaces)
Updating Resistify to v0.5.0, switching source to GitHub as PyPI file size is now exceeded. Added additional dependencies and misc.
Please read the guidelines for Bioconda recipes before opening a pull request (PR).
General instructions
@BiocondaBot please add labelcommand.@bioconda/corein a comment.Instructions for avoiding API, ABI, and CLI breakage issues
Conda is able to record and lock (a.k.a. pin) dependency versions used at build time of other recipes.
This way, one can avoid that expectations of a downstream recipe with regards to API, ABI, or CLI are violated by later changes in the recipe.
If not already present in the meta.yaml, make sure to specify
run_exports(see here for the rationale and comprehensive explanation).Add a
run_exportssection like this:with
...being one of:{{ pin_subpackage("myrecipe", max_pin="x") }}{{ pin_subpackage("myrecipe", max_pin="x.x") }}{{ pin_subpackage("myrecipe", max_pin="x.x") }}(in such a case, please add a note that shortly mentions your evidence for that){{ pin_subpackage("myrecipe", max_pin="x.x.x") }}(in such a case, please add a note that shortly mentions your evidence for that){{ pin_subpackage("myrecipe", max_pin=None) }}while replacing
"myrecipe"with eithernameif aname|lowervariable is defined in your recipe or with the lowercase name of the package in quotes.Bot commands for PR management
Please use the following BiocondaBot commands:
Everyone has access to the following BiocondaBot commands, which can be given in a comment:
@BiocondaBot please update@BiocondaBot please add labelplease review & mergelabel.@BiocondaBot please fetch artifactsYou can use this to test packages locally.
Note that the
@BiocondaBot please mergecommand is now depreciated. Please just squash and merge instead.Also, the bot watches for comments from non-members that include
@bioconda/<team>and will automatically re-post them to notify the addressed<team>.