Update PhaBOX to 2.1.4 #51830
Update PhaBOX to 2.1.4 #51830
Conversation
Update PhaBOX to 2.1.2 version and add new requirements
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces updates to the Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (1)
recipes/phabox/meta.yaml (1)
26-36: Remove trailing spaces.Several lines contain trailing spaces. While this doesn't affect functionality, it's good practice to remove them for consistency.
Apply this formatting fix to remove trailing spaces from the affected lines.
Also applies to: 53-53
🧰 Tools
🪛 yamllint
[error] 28-28: trailing spaces
(trailing-spaces)
[error] 29-29: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
recipes/phabox/meta.yaml(1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/phabox/meta.yaml
[error] 28-28: trailing spaces
(trailing-spaces)
[error] 29-29: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 53-53: trailing spaces
(trailing-spaces)
🔇 Additional comments (3)
recipes/phabox/meta.yaml (3)
50-50: LGTM! Documentation updates look good.
The updated home URL and summary provide clearer information about the project.
Also applies to: 54-54
24-42: 🛠️ Refactor suggestion
Consider relaxing Python version constraint.
The Python version is strictly pinned to 3.10.14. Consider using a more flexible version constraint (e.g., python >=3.10,<3.11) to allow for minor version updates while maintaining compatibility.
Verify ML dependencies compatibility.
New ML-related dependencies (transformers, pytorch, triton, accelerate) have been added without version constraints. This could lead to compatibility issues.
#!/bin/bash
# Check if these dependencies are commonly used together in other bioconda recipes
rg -l "transformers.*pytorch.*triton.*accelerate" --type yaml🧰 Tools
🪛 yamllint
[error] 28-28: trailing spaces
(trailing-spaces)
[error] 29-29: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
2-2: Verify SHA256 checksum for the new version.
The version update and URL format look good, but let's verify the SHA256 checksum matches the source archive.
Also applies to: 9-10
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
recipes/phabox/meta.yaml (1)
26-36: Clean up trailing spaces.There are trailing spaces in several lines that should be removed for consistency.
Apply this diff to clean up trailing spaces:
- - blast =2.16.0 - - fasttree =2.1.11 - - kcounter =0.1.1 - - mafft =7.525 - - numpy =1.26.4 - - pandas =2.2.3 - - networkx =3.4.2 - - seaborn-base =0.13.2 - - biopython =1.84 - - tqdm =4.66.5 - license_file: "LICENSE.md" + - blast =2.16.0 + - fasttree =2.1.11 + - kcounter =0.1.1 + - mafft =7.525 + - numpy =1.26.4 + - pandas =2.2.3 + - networkx =3.4.2 + - seaborn-base =0.13.2 + - biopython =1.84 + - tqdm =4.66.5 + license_file: "LICENSE.md"Also applies to: 53-53
🧰 Tools
🪛 yamllint
[error] 28-28: trailing spaces
(trailing-spaces)
[error] 29-29: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
recipes/phabox/meta.yaml(1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/phabox/meta.yaml
[error] 28-28: trailing spaces
(trailing-spaces)
[error] 29-29: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 53-53: trailing spaces
(trailing-spaces)
🔇 Additional comments (5)
recipes/phabox/meta.yaml (5)
2-10: LGTM: Version update and source configuration look correct.
The version bump to 2.1.2 and corresponding source URL/checksum updates are properly configured.
12-17: LGTM: Build configuration is properly set up.
The build configuration follows best practices with correct noarch python setting and proper entry point definition.
49-56: LGTM: Documentation and metadata are well configured.
The about section is properly updated with correct GitHub repository links and a clear, concise summary.
🧰 Tools
🪛 yamllint
[error] 53-53: trailing spaces
(trailing-spaces)
58-64: LGTM: Extra metadata is properly configured.
The maintainers list and identifiers are correctly updated.
19-43: Consider relaxing some version constraints.
The current configuration has very strict version pins for most dependencies. While exact version pins can prevent compatibility issues, they may also:
- Make it harder to maintain the package
- Limit users' flexibility in their conda environments
- Increase conflicts with other packages
Consider using more flexible version constraints where possible, especially for Python (e.g., python >=3.10,<3.11 instead of =3.10.14).
✅ Verification successful
Let me gather more specific information about version requirements in the phabox package itself.
Let me try one more verification to check the package's source code for any hard dependencies.
Consider using more flexible version constraints
Based on the verification, there's no evidence in the package's source code or metadata that requires these exact version pins. The strict version constraints appear to be overly restrictive. Recommend:
- Using a more flexible Python constraint like
python >=3.10,<3.11 - Using minimum version constraints for core dependencies (e.g.,
numpy >=1.26,pandas >=2.2) - Keeping exact pins only for dependencies with known compatibility issues
This will improve maintainability and reduce potential conflicts in user environments while still ensuring compatibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if these exact versions are required by searching the upstream repository
rg -l "3\.10\.14|1\.26\.4|2\.2\.3|3\.4\.2|0\.13\.2|1\.84|4\.66\.5|1\.14\.1"
Length of output: 6288
Script:
#!/bin/bash
# Check the repository for any specific version requirements in setup.py, requirements.txt or similar files
fd -e py -e txt -e yaml "setup|requirements|environment" --exec rg -l "numpy|pandas|networkx|seaborn|biopython|tqdm|scipy|python.*3\.10"
Length of output: 200
Script:
#!/bin/bash
# Look for import statements and version checks in the package
rg -A 2 "import (numpy|pandas|networkx|seaborn|biopython|scipy)" recipes/phabox/
rg "require|version" recipes/phabox/
Length of output: 379
🧰 Tools
🪛 yamllint
[error] 28-28: trailing spaces
(trailing-spaces)
[error] 29-29: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
|
@BiocondaBot please add label |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
recipes/phabox/meta.yaml (2)
45-47: Consider enhancing test coverage.The current test only verifies that the help command works. Consider adding basic functionality tests or version check.
test: commands: - phabox2 --help + - phabox2 --version + - python -c "import phabox2; assert phabox2.__version__ == '2.1.2'"
Line range hint
60-65: LGTM! Clean up trailing spaces.The maintainers and identifiers are properly updated.
Please remove trailing spaces from the file (lines 28, 29, 31-36, 53) to satisfy the linter.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
[error] 28-28: trailing spaces
(trailing-spaces)
[error] 29-29: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 53-53: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
recipes/phabox/meta.yaml(1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/phabox/meta.yaml
[error] 28-28: trailing spaces
(trailing-spaces)
[error] 29-29: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 53-53: trailing spaces
(trailing-spaces)
🔇 Additional comments (3)
recipes/phabox/meta.yaml (3)
2-2: LGTM! Version and source information properly updated.
The version update to 2.1.2 and corresponding source URL changes are consistent with the upstream repository tags.
Also applies to: 9-10
15-15: LGTM! Build configuration follows best practices.
The simplified pip install command and updated entry point are appropriate for this package.
Also applies to: 17-17
50-56: LGTM! Documentation links and metadata are well organized.
The GitHub repository links and concise summary improve the package metadata.
🧰 Tools
🪛 yamllint
[error] 53-53: trailing spaces
(trailing-spaces)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
recipes/phabox/meta.yaml (1)
27-37: Remove trailing spaces.Several lines contain trailing spaces. While this doesn't affect functionality, it's good practice to maintain clean YAML files.
Also applies to: 54-54
🧰 Tools
🪛 yamllint
[error] 29-29: trailing spaces
(trailing-spaces)
[error] 30-30: trailing spaces
(trailing-spaces)
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 37-37: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
recipes/phabox/meta.yaml(1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/phabox/meta.yaml
[error] 29-29: trailing spaces
(trailing-spaces)
[error] 30-30: trailing spaces
(trailing-spaces)
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 37-37: trailing spaces
(trailing-spaces)
[error] 54-54: trailing spaces
(trailing-spaces)
🔇 Additional comments (5)
recipes/phabox/meta.yaml (5)
12-19: Build configuration looks good!
The build configuration follows bioconda best practices with appropriate noarch Python package setup and entry point configuration.
25-43: Previous review comments about dependency versions are still valid.
The concerns about Python version constraint and missing ML dependency versions from the previous review haven't been addressed yet.
🧰 Tools
🪛 yamllint
[error] 29-29: trailing spaces
(trailing-spaces)
[error] 30-30: trailing spaces
(trailing-spaces)
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 37-37: trailing spaces
(trailing-spaces)
51-57: About section updates look good!
The updated URLs and summary provide better clarity about the package's purpose and resources.
🧰 Tools
🪛 yamllint
[error] 54-54: trailing spaces
(trailing-spaces)
Line range hint 61-66: Extra section updates look good!
The maintainers list and identifiers are properly updated.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
[error] 29-29: trailing spaces
(trailing-spaces)
[error] 30-30: trailing spaces
(trailing-spaces)
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 37-37: trailing spaces
(trailing-spaces)
[error] 54-54: trailing spaces
(trailing-spaces)
9-9:
Add SHA256 checksum for source verification.
The SHA256 checksum is missing for the source tarball. This is important for ensuring the integrity of the downloaded package.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
recipes/phabox/meta.yaml (2)
26-45: Consider using version ranges instead of exact pins.While exact version pins provide reproducibility, they might be too restrictive for conda's dependency solver. Consider using version ranges (e.g.,
>=1.26.4,<2) for dependencies that don't strictly require exact versions.This allows for more flexibility while maintaining compatibility:
- - numpy =1.26.4 - - pandas =2.2.3 - - networkx =3.4.2 - - seaborn-base =0.13.2 - - biopython =1.84 + - numpy >=1.26.4,<2 + - pandas >=2.2.0,<3 + - networkx >=3.4.0,<4 + - seaborn-base >=0.13.0,<0.14 + - biopython >=1.84,<2🧰 Tools
🪛 yamllint
[error] 30-30: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 37-37: trailing spaces
(trailing-spaces)
[error] 38-38: trailing spaces
(trailing-spaces)
28-38: Remove trailing spaces.Several lines contain trailing spaces. While this doesn't affect functionality, it's good practice to maintain clean YAML files.
Also applies to: 55-55
🧰 Tools
🪛 yamllint
[error] 30-30: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 37-37: trailing spaces
(trailing-spaces)
[error] 38-38: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
recipes/phabox/meta.yaml(1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/phabox/meta.yaml
[error] 30-30: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 36-36: trailing spaces
(trailing-spaces)
[error] 37-37: trailing spaces
(trailing-spaces)
[error] 38-38: trailing spaces
(trailing-spaces)
[error] 55-55: trailing spaces
(trailing-spaces)
🔇 Additional comments (3)
recipes/phabox/meta.yaml (3)
17-19: LGTM! Build configuration is appropriate.
The build configuration is correctly set up with:
- Appropriate entry point
- Correct run_exports configuration
2-2: Verify source URL accessibility.
The URL format has been updated to include the 'v' prefix. Let's verify the URL is accessible.
Also applies to: 9-10
✅ Verification successful
Let me try a different approach to verify the URL accessibility.
Source URL and tag v2.1.3 are valid
The GitHub API confirms that tag v2.1.3 exists in the repository with commit SHA 72c2251, which means the source URL in the recipe is valid and accessible. The URL format with 'v' prefix is correct and matches the repository's tag structure.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the source URL is accessible
URL="https://github.com/KennthShang/PhaBOX/archive/refs/tags/v2.1.3.tar.gz"
curl -I $URL
Length of output: 1130
Script:
#!/bin/bash
# Verify the source URL is accessible using wget instead of curl
# -S shows error messages
# --spider mode only checks existence without downloading
# --no-check-certificate to avoid SSL verification issues
wget -S --spider --no-check-certificate "https://github.com/KennthShang/PhaBOX/archive/refs/tags/v2.1.3.tar.gz" 2>&1
# Also verify if the tag exists in the repository
gh api repos/KennthShang/PhaBOX/git/refs/tags/v2.1.3
Length of output: 576
62-63: Verify maintainer permissions.
New maintainers have been added. Let's verify they have the necessary permissions on the repository.
|
@bgruening Sorry for bothering. I wonder if it will take a long time to test and if I will need a new approval every time I revise the script. Thanks. Also, it seems I can easily pass the conda build and upload my package to the Anaconda cloud (my personal account), but I usually fail the Bioconda test. Do you have any suggestions? Best, |
Yes, an approval is needed for every pull request.
Bioconda has stricter testing. Many people and projects only use bioconda and conda-forge, because these are quite strictly tested and well maintained sources of software. In my daily practice I would never install software from other conda channels. bioconda comes with some additional effort, but it certainly pays off on the longer run (IMO). |
|
I restored the old version pinning (and re-changed the order of the requirements .. which makes it easier to track changes). Lets see if this works. The strict pinning of your commits could not be resolver by conda (wrt the pinning given by the bioconda ecosystem) |
|
Thanks for the advice. Since it still failed to pass the test, I made some revisions. I think the major reason is the version of the deep learning packages. I left the version as blank to avoid conflict when building my packages, so let's see whether it works here. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
recipes/phabox/meta.yaml (2)
28-29: Consider relaxing some version constraints.Some dependencies have very strict version pins which might cause unnecessary conflicts in the conda dependency resolver. Consider relaxing version constraints where exact versions aren't critical:
- - biopython >=1.84 - - networkx >=3.4.2 - - numpy >=1.26.4 - - pandas >=2.2.3 - - scipy >=1.14.1 - - diamond <=0.9.14 + - biopython >=1.84,<2.0 + - networkx >=3.4 + - numpy >=1.26 + - pandas >=2.2 + - scipy >=1.14 + - diamond >=0.9.14Also applies to: 33-35, 38-38, 41-44
53-57: LGTM: About section updates are appropriate.The home URL update and summary changes are well-aligned with the project's current state. Note: There's a trailing space in the license_file line that should be removed.
🧰 Tools
🪛 yamllint
[error] 56-56: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
recipes/phabox/meta.yaml(1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/phabox/meta.yaml
[error] 56-56: trailing spaces
(trailing-spaces)
🔇 Additional comments (3)
recipes/phabox/meta.yaml (3)
2-2: LGTM: Package metadata updates are correct.
The version update to 2.1.3 and corresponding source URL changes with SHA256 update are properly implemented.
Also applies to: 9-10
17-17: LGTM: Build configuration is appropriate.
The entry point update and build configuration follow bioconda best practices.
63-64: LGTM: Maintainer updates and identifiers are appropriate.
The maintainer list update properly reflects the current contributors.
|
I got a very strange error during the test: So I revised script and try again. I thought this could be caused by the size of my released package. Will try to fix it after I revise the os.system() in the source codes. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
recipes/phabox/meta.yaml (1)
56-56: Fix trailing space in license_file line.Remove the trailing space after "LICENSE.md".
- license_file: "LICENSE.md" + license_file: "LICENSE.md"🧰 Tools
🪛 yamllint
[error] 56-56: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
recipes/phabox/meta.yaml(1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/phabox/meta.yaml
[error] 56-56: trailing spaces
(trailing-spaces)
🔇 Additional comments (3)
recipes/phabox/meta.yaml (3)
2-2: LGTM: Version update and source configuration look good.
The version update to 2.1.3 and corresponding source URL changes with SHA256 update are properly configured.
Also applies to: 9-10
17-17: LGTM: Build configuration and entry point are properly set up.
The build configuration follows best practices, and the entry point is correctly defined.
53-53: LGTM: Package metadata updates are appropriate.
The home URL, summary, and documentation links are properly updated to reflect the current project state.
Also applies to: 57-58
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/phabox/meta.yaml(1 hunks)
🔇 Additional comments (5)
recipes/phabox/meta.yaml (5)
2-2: LGTM: Version update and source configuration look good!
The version bump to 2.1.3 and corresponding source URL changes follow proper conventions.
Also applies to: 9-10
37-46: Review ML dependencies version constraints.
The ML-related packages lack version constraints which could lead to compatibility issues:
- - pytorch
- - transformers
- - triton
- - accelerate
+ - pytorch >=2.1.0
+ - transformers >=4.34.0
+ - triton >=2.0.0
+ - accelerate >=0.27.053-59: LGTM: Documentation links and metadata updates are appropriate.
The changes to documentation URLs and summary provide better accessibility and clarity.
14-14: Consider build isolation and verify entry point change.
- The
--no-build-isolationflag could lead to dependency resolution issues. Consider if this is necessary. - The entry point has changed to
phabox2. Let's verify this change is intentional and documented.
Also applies to: 17-17
✅ Verification successful
Build isolation and entry point changes are intentional and correct
The review comment's concerns have been investigated and can be addressed:
-
The
--no-build-isolationflag is used intentionally in the build script along with--no-deps, which is a common pattern in conda-forge recipes when dependencies are managed through conda's requirements section. -
The entry point
phabox2is correctly configured in meta.yaml and matches the package structure (phabox2.phabox2:main). This appears to be the intended entry point for version 2.x of the package.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the entry point change is documented
rg -l 'phabox2' --type md
Length of output: 25
Script:
#!/bin/bash
# Check entry point configuration in setup files and build scripts
rg -l "phabox2"
# Check build isolation flag usage
rg --no-build-isolation
# Look for setup.py or pyproject.toml to verify entry point configuration
fd "setup.py|pyproject.toml" --exec cat {}
Length of output: 43208
Script:
#!/bin/bash
# Check build script for build isolation flag
cat recipes/phabox/build.sh
# Check for entry point configuration in recipe
cat recipes/phabox/meta.yaml
# Look for phabox2 references in source code
rg -l "phabox2"
Length of output: 1631
63-64: Verify maintainer changes.
Please ensure that:
- The original maintainer has approved these changes
- The new maintainers have the necessary permissions and access
✅ Verification successful
Maintainer changes appear to be authorized
Based on the git history and GitHub contributors list:
- SHANG Jiayu (KennthShang) is the original author and active contributor
- PENG Cheng (ChengPENG-wolf) is a verified contributor to the project
- Both maintainers have demonstrated involvement with the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if new maintainers have contributed to the project
gh api -X GET /repos/KennthShang/PhaBOX/contributors --jq '.[].login'
Length of output: 98
Script:
#!/bin/bash
# Check if the PR author is one of the maintainers or has necessary permissions
gh pr view --json author,reviewDecision,reviews,commits | jq -r '.author.login, .reviewDecision, .reviews[].author.login, (.commits[].authors[].user.login | select(. != null))'
Length of output: 231
Script:
#!/bin/bash
# Check git history for the specific file to understand who made these changes
git log --follow --pretty=format:"%h %ae %an" recipes/phabox/meta.yaml
# Also check the commit messages for any approval or authorization information
git log --follow --pretty=format:"%h %s" recipes/phabox/meta.yaml
Length of output: 918
|
I removed all the unnecessary files in the release and all the os.system() are replaced by subprocess.run(). Provides standard error and return a non-zero exit code in case one of the calls fails in this version |
|
Seems the |
remove all independent packages
|
Are you downloading / packaging some kind of data as well? Packages have never been a problem for the space, we have recipes with much much larger packages. So somewhere there needs to be (references, model) data be involved. |
|
Seems not to be the case. The zip is only 6MB, but a few pyc files might be removed: KennthShang/PhaBOX#40 |
|
@bernt-matthias I have removed them now. @bgruening Do not know why, but after I followed the original rules (receipt from @bernt-matthias ) and added new packages below there, it worked well #51958. So maybe it is caused by the downloaded dependencies during the build. |
Update PhaBOX to the 2.1.3 version and add new requirements