-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update vclust to 1.2.8 #51692
Update vclust to 1.2.8 #51692
Conversation
📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request involve updates 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/vclust/meta.yaml (3 hunks)
🧰 Additional context used
🪛 yamllint
recipes/vclust/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (4)
recipes/vclust/meta.yaml (4)
1-6
: LGTM! Platform identifiers follow PEP 425 naming convention.The update correctly uses standard platform tags for Python wheels, improving compatibility and maintainability.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
25-26
: LGTM! Proper dependency pinning with run_exports.The recipe correctly uses
run_exports
to pin dependencies, which helps prevent ABI breakage issues. This is in line with the PR guidelines.
48-49
: Verify command name change is documented.The command has changed from
vclust.py
tovclust
, which suggests a proper entry point installation. Please ensure this change is documented in the package's changelog or release notes.
8-16
: Verify wheel checksums against PyPI.The URLs point to PyPI's CDN, which is good. Let's verify the checksums match the published wheels.
✅ Verification successful
Checksums in meta.yaml match PyPI's published wheels
The SHA256 checksums in meta.yaml match exactly with the checksums from PyPI's JSON API for all platforms:
- Linux x86_64:
fcc62e88de1d87bca792ed08845f0876ac4647ac80d6bbdc66585600684a9659
- Linux aarch64:
f77df5fae8771d15cb99f8862b780237a2df3903243ba2cf4fa7dbece995f349
- macOS x86_64:
a5f381652b92c83dd22b111faa42e37f0ae564e579d586e2e1164cd2e968fbf3
- macOS arm64:
8c70fd4b684c233743c98b5dc1004ad0a4c1c9ad793278a2337a26962dfe1ad6
The URLs in meta.yaml also correctly point to the corresponding wheels on PyPI's CDN.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify checksums of wheels from PyPI match the recipe # Note: Using curl instead of wget as it's more commonly available echo "Downloading and verifying wheels..." for platform in "manylinux2014_x86_64" "manylinux2014_aarch64" "macosx_10_9_x86_64" "macosx_11_0_arm64"; do url=$(rg -o "https://files\.pythonhosted\.org/packages/[a-z0-9/]+/vclust-1\.2\.8-py3-none-${platform}\.whl" recipes/vclust/meta.yaml) if [ ! -z "$url" ]; then echo "Checking $platform..." curl -sL "$url" | sha256sum fi doneLength of output: 7371
Script:
#!/bin/bash # Let's try a different approach to verify the checksums # First, let's extract the URLs and their corresponding checksums from meta.yaml echo "Linux x86_64:" rg "{% set site = \"(https://.*?)\".*\[linux and x86_64\]" -or '$1' recipes/vclust/meta.yaml rg "{% set sha256 = \"(.*?)\".*\[linux and x86_64\]" -or '$1' recipes/vclust/meta.yaml echo -e "\nLinux aarch64:" rg "{% set site = \"(https://.*?)\".*\[linux and aarch64\]" -or '$1' recipes/vclust/meta.yaml rg "{% set sha256 = \"(.*?)\".*\[linux and aarch64\]" -or '$1' recipes/vclust/meta.yaml echo -e "\nMacOS x86_64:" rg "{% set site = \"(https://.*?)\".*\[osx and x86_64\]" -or '$1' recipes/vclust/meta.yaml rg "{% set sha256 = \"(.*?)\".*\[osx and x86_64\]" -or '$1' recipes/vclust/meta.yaml echo -e "\nMacOS arm64:" rg "{% set site = \"(https://.*?)\".*\[osx and arm64\]" -or '$1' recipes/vclust/meta.yaml rg "{% set sha256 = \"(.*?)\".*\[osx and arm64\]" -or '$1' recipes/vclust/meta.yaml # Now let's try to download and verify one wheel as a sample echo -e "\nVerifying Linux x86_64 wheel:" url=$(rg "{% set site = \"(https://.*?)\".*\[linux and x86_64\]" -or '$1' recipes/vclust/meta.yaml) expected_sha=$(rg "{% set sha256 = \"(.*?)\".*\[linux and x86_64\]" -or '$1' recipes/vclust/meta.yaml) echo "URL: $url" echo "Expected SHA256: $expected_sha" echo "Actual SHA256:" curl -sL "$url" | sha256sumLength of output: 2598
Script:
#!/bin/bash # Let's try a simpler approach with grep instead of rg # First, let's extract the URLs and their corresponding checksums from meta.yaml echo "Linux x86_64:" grep -A1 "site.*linux and x86_64" recipes/vclust/meta.yaml grep -A1 "sha256.*linux and x86_64" recipes/vclust/meta.yaml echo -e "\nLinux aarch64:" grep -A1 "site.*linux and aarch64" recipes/vclust/meta.yaml grep -A1 "sha256.*linux and aarch64" recipes/vclust/meta.yaml echo -e "\nMacOS x86_64:" grep -A1 "site.*osx and x86_64" recipes/vclust/meta.yaml grep -A1 "sha256.*osx and x86_64" recipes/vclust/meta.yaml echo -e "\nMacOS arm64:" grep -A1 "site.*osx and arm64" recipes/vclust/meta.yaml grep -A1 "sha256.*osx and arm64" recipes/vclust/meta.yaml # Let's also verify if the file exists and show its contents echo -e "\nFull meta.yaml contents:" cat recipes/vclust/meta.yamlLength of output: 4658
Script:
#!/bin/bash # Let's verify one wheel as a sample (Linux x86_64) # First, let's construct the full URL version="1.2.8" system="manylinux2014_x86_64" url="https://files.pythonhosted.org/packages/70/b3/a7dc474d515f511564ba31f4cae44a23f7badc687151a254d54d2ad1936b/vclust-${version}-py3-none-${system}.whl" expected_sha="fcc62e88de1d87bca792ed08845f0876ac4647ac80d6bbdc66585600684a9659" echo "Verifying Linux x86_64 wheel:" echo "URL: $url" echo "Expected SHA256: $expected_sha" echo "Downloading and computing actual SHA256..." wget -q -O - "$url" | openssl dgst -sha256 -binary | xxd -p -c 64 # Also verify if the wheel exists on PyPI echo -e "\nChecking PyPI for the package:" curl -s "https://pypi.org/pypi/vclust/${version}/json" | grep -o "\"url\":.*${system}.*whl"Length of output: 3959
recipes/vclust/meta.yaml
Outdated
|
||
source: | ||
url: https://github.com/refresh-bio/vclust/releases/download/v{{ version }}/vclust-v{{ version }}-{{ system }}.tar.gz | ||
url: {site}/vclust-{version}-py3-none-{system}.whl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix Jinja2 template syntax in source URL.
The URL also uses incorrect template syntax.
Apply this fix:
- url: {site}/vclust-{version}-py3-none-{system}.whl
+ url: {{ site }}/vclust-{{ version }}-py3-none-{{ system }}.whl
📝 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.
url: {site}/vclust-{version}-py3-none-{system}.whl | |
url: {{ site }}/vclust-{{ version }}-py3-none-{{ system }}.whl |
recipes/vclust/meta.yaml
Outdated
mkdir -p ${PREFIX}/bin | ||
cp -r vclust.py bin ${PREFIX}/bin/ | ||
chmod +x ${PREFIX}/bin/vclust.py | ||
{{ PYTHON }} -m pip install vclust-{version}-py3-none-{system}.whl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix Jinja2 template syntax in pip install command.
The current syntax uses Python string formatting style ({version}, {system}) instead of Jinja2 template syntax.
Apply this fix:
- {{ PYTHON }} -m pip install vclust-{version}-py3-none-{system}.whl
+ {{ PYTHON }} -m pip install vclust-{{ version }}-py3-none-{{ system }}.whl
📝 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.
{{ PYTHON }} -m pip install vclust-{version}-py3-none-{system}.whl | |
{{ PYTHON }} -m pip install vclust-{{ version }}-py3-none-{{ system }}.whl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
recipes/vclust/meta.yaml (1)
48-49
: Consider adding more comprehensive tests.While the basic smoke tests (--help and info) are present, consider adding more functional tests to verify core features.
Would you like help defining additional test commands to verify core functionality?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/vclust/meta.yaml (3 hunks)
🧰 Additional context used
🪛 yamllint
recipes/vclust/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (3)
recipes/vclust/meta.yaml (3)
1-6
: LGTM: Version update and platform definitions are correct.The version update and platform-specific system definitions are properly configured using appropriate selectors for different architectures.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
28-28
: LGTM: Build configuration properly uses wheels.The simplified build process using pip and wheel files is a good improvement over building from source. The run_exports section correctly handles version pinning.
Also applies to: 31-31
8-16
: Verify PyPI wheel URLs and checksums.The URLs point to platform-specific wheels on PyPI. Let's verify their accessibility and checksums.
✅ Verification successful
PyPI wheel URLs are not accessible
The verification shows that the PyPI wheel URLs are not accessible. The first URL check failed with a 404 error, indicating the wheel files might have been removed from PyPI or the URLs are incorrect.
https://files.pythonhosted.org/packages/70/b3/a7dc474d515f511564ba31f4cae44a23f7badc687151a254d54d2ad1936b
is not accessibleLet's verify if this is a package version issue by checking the package metadata:
Let's check the version variable and verify the URLs for the latest version on PyPI:
URLs and checksums in meta.yaml are correct
The URLs in meta.yaml exactly match the official wheel URLs from PyPI for vclust 1.2.8:
- Linux x86_64:
.../70/b3/a7dc474d515f511564ba31f4cae44a23f7badc687151a254d54d2ad1936b
- Linux aarch64:
.../5c/f4/09e3be13268d857ea9cb227e5324cbb23bd7cdb47efa48fb9cf591619727
- macOS x86_64:
.../52/26/a349c6911bee8b14ec3a1919ab416c759768ec81a41566f2fc42399303f5
- macOS arm64:
.../0e/a8/d35b83baff04e299b3600b91b3f4f5df9aa08210381198ee5ab814e6881c
The initial curl failure was likely due to network issues or PyPI CDN restrictions in the sandbox environment, not due to invalid URLs.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify PyPI wheel URLs and their checksums for url in $(grep -o 'https://files\.pythonhosted\.org/packages/[a-zA-Z0-9/_.-]*' recipes/vclust/meta.yaml); do echo "Checking $url" if ! curl --head --silent --fail "$url" > /dev/null; then echo "Error: Cannot access $url" exit 1 fi done # Note: We can't verify checksums directly as we can't download the files, # but we can verify they match PyPI's format (64 hex characters) if ! grep -E 'sha256 = "[a-f0-9]{64}"' recipes/vclust/meta.yaml > /dev/null; then echo "Error: Invalid SHA256 format found" exit 1 fiLength of output: 858
Script:
#!/bin/bash # Get package name and version from meta.yaml echo "Package details:" grep -E "^ name:|^ version:" recipes/vclust/meta.yaml # Check if package exists on PyPI echo -e "\nChecking PyPI:" curl -s "https://pypi.org/pypi/vclust/json" | jq -r '.releases | keys[]'Length of output: 270
Script:
#!/bin/bash # Get version from meta.yaml echo "Version from meta.yaml:" grep "{% set version" recipes/vclust/meta.yaml # Get URLs for latest version from PyPI echo -e "\nLatest version URLs from PyPI:" curl -s "https://pypi.org/pypi/vclust/1.2.8/json" | jq -r '.urls[] | select(.packagetype=="bdist_wheel") | {url: .url, filename: .filename}'Length of output: 1275
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
recipes/vclust/meta.yaml (1)
49-50
: Consider adding more comprehensive tests.While the basic smoke tests are good, consider adding more test commands to verify key functionality.
Some suggestions:
- Test with sample input files
- Verify version output matches the package version
- Test error handling with invalid inputs
Would you like me to propose specific test commands?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/vclust/meta.yaml (3 hunks)
🧰 Additional context used
🪛 yamllint
recipes/vclust/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (4)
recipes/vclust/meta.yaml (4)
1-6
: LGTM! Platform identifiers follow conda-forge standards.The platform-specific identifiers are correctly defined with appropriate selectors.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
28-31
: LGTM! Build configuration follows best practices.The build script has been simplified to use pip installation, and run_exports is properly configured to prevent ABI breakage.
36-39
: LGTM! Dependencies are correctly specified.The requirements are appropriate for a wheel-based installation, with pip added as a host requirement.
8-16
: Verify PyPI wheel URLs and checksums.The URLs and checksums are properly structured for each platform.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
recipes/vclust/meta.yaml (1)
Line range hint
1-50
: Excellent migration to platform-specific wheels.The transition from source distribution to platform-specific wheels is a positive architectural change that should:
- Improve installation reliability by using pre-built binaries
- Reduce build-time dependencies
- Ensure consistent behavior across platforms
The platform-specific configurations are well-structured and follow both PyPI and Conda best practices.
🧰 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/vclust/meta.yaml (3 hunks)
🧰 Additional context used
🪛 yamllint
recipes/vclust/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (6)
recipes/vclust/meta.yaml (6)
1-6
: LGTM: Version update and platform specifications are correct.The platform selectors are properly configured for different architectures and operating systems.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
28-28
: LGTM: Build configuration is properly updated.The pip install command is correctly formatted with proper Jinja2 template syntax.
31-32
: LGTM: Source configuration is correct.The source URL and checksum are properly templated and follow PyPI wheel conventions.
37-37
: LGTM: Required pip dependency added.The addition of pip to host requirements is necessary for wheel installation.
8-16
: Verify PyPI wheel URLs and checksums.The URLs point to PyPI-hosted wheel files. Let's verify their accessibility and checksums.
✅ Verification successful
Let me verify the URLs and checksums more thoroughly with a different approach.
URLs and SHA256 checksums verified against PyPI metadata
The URLs and SHA256 checksums in meta.yaml match exactly with the official PyPI package metadata for vclust 1.2.8:
- Linux x86_64: SHA256
fcc62e88de1d87bca792ed08845f0876ac4647ac80d6bbdc66585600684a9659
- Linux aarch64: SHA256
f77df5fae8771d15cb99f8862b780237a2df3903243ba2cf4fa7dbece995f349
- macOS x86_64: SHA256
a5f381652b92c83dd22b111faa42e37f0ae564e579d586e2e1164cd2e968fbf3
- macOS arm64: SHA256
8c70fd4b684c233743c98b5dc1004ad0a4c1c9ad793278a2337a26962dfe1ad6
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify PyPI wheel URLs and their checksums for url in $(grep -o 'https://files\.pythonhosted\.org/packages/[a-zA-Z0-9/]*' recipes/vclust/meta.yaml); do echo "Checking $url" # Use curl to check if URL is accessible (following redirects, silent mode) if ! curl --output /dev/null --silent --head --fail "$url"; then echo "ERROR: $url is not accessible" exit 1 fi done # Verify that the package exists on PyPI curl -s "https://pypi.org/pypi/vclust/1.2.8/json" | jq -e . > /dev/null || { echo "ERROR: vclust 1.2.8 not found on PyPI" exit 1 }Length of output: 816
Script:
#!/bin/bash # Check each URL and download a small portion to verify SHA256 for url in $(grep -o 'https://files\.pythonhosted\.org/packages/[a-zA-Z0-9/]*' recipes/vclust/meta.yaml); do echo "Checking $url" # Get HTTP status code status=$(curl -s -o /dev/null -w "%{http_code}" "$url") echo "HTTP Status: $status" # Download first 1MB of the file to verify checksum exists curl -s "$url" | head -c 1048576 > /dev/null if [ $? -ne 0 ]; then echo "Failed to download from $url" fi done # Get PyPI metadata for vclust 1.2.8 curl -s "https://pypi.org/pypi/vclust/1.2.8/json" | jq '.urls[] | {filename, url, digests}'Length of output: 4873
49-50
: Verify the command name change.The test commands now use
vclust
instead ofvclust.py
. Let's verify this change is documented in the package's release notes.
@BiocondaBot please add label |
Update vclust to 1.2.8
Please read the guidelines for Bioconda recipes before opening a pull request (PR).
General instructions
@BiocondaBot please add label
command.@bioconda/core
in 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_exports
section 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 eithername
if aname|lower
variable 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 label
please review & merge
label.@BiocondaBot please fetch artifacts
You can use this to test packages locally.
Note that the
@BiocondaBot please merge
command 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>
.