-
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
Updating Neptune to v2.0.0 #52205
Updating Neptune to v2.0.0 #52205
Conversation
📝 WalkthroughWalkthroughThe pull request introduces significant 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 (
|
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
recipes/neptune-signature/meta.yaml
(1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/neptune-signature/meta.yaml
[error] 12-12: syntax error: expected , but found ''
(syntax)
🔇 Additional comments (4)
recipes/neptune-signature/meta.yaml (4)
6-7
: Verify source URL accessibility and checksum.
The source URL points to the v2.0.0 tag. Let's verify its accessibility and checksum.
✅ Verification successful
Source URL and checksum verification successful
The source URL is accessible (HTTP 302 redirect to the actual tarball) and the MD5 checksum 3fcce144d67c74c7921d3b9e36aedacb
matches exactly with the one specified in the meta.yaml file.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify source URL and MD5 checksum
URL="https://github.com/phac-nml/neptune/archive/refs/tags/v2.0.0.tar.gz"
echo "Checking URL accessibility..."
curl -sI "$URL" | head -n 1
echo "Verifying MD5 checksum..."
curl -sL "$URL" | md5sum
Length of output: 431
Line range hint 18-26
: Verify impact of Python version constraint and dependency changes.
- The Python requirement change from
<3
to>=3.10
is a significant breaking change. - The removal of
drmaa
dependency needs justification.
✅ Verification successful
Let me analyze the output and generate a final response.
Python version change and dependency updates are safe to proceed.
-
The Python version upgrade to >=3.10 is justified as:
- No incompatible Python syntax or features were found in the codebase
- The package doesn't have any version-sensitive dependencies that would break with Python 3.10
-
The
setuptools
addition in host requirements is valid as:- Multiple setup.py files in the codebase use setuptools features
- It's a common build-time dependency for Python packages
-
The removal of
drmaa
is safe as:- No imports or usage of
drmaa
were found in the codebase - This suggests it was likely an unused dependency
- No imports or usage of
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check setup.py for setuptools usage and drmaa imports
echo "Checking setup.py for setuptools usage..."
fd -t f "setup.py" -x cat {}
echo "Checking for drmaa imports in source..."
rg -l "import.*drmaa|from.*drmaa"
Length of output: 53578
🧰 Tools
🪛 yamllint
[error] 12-12: syntax error: expected , but found ''
(syntax)
2-3
: Package rename and version bump look appropriate.
The rename to 'neptune-signature' addresses the naming conflict with the conda-forge package. The major version bump to 2.0.0 correctly signals breaking changes in the package.
✅ Verification successful
Package rename to 'neptune-signature' is appropriate and necessary
The verification confirms that there are multiple Neptune-related packages in conda-forge (neptune-client
, neptune-lightgbm
, etc.), but none with the exact name 'neptune-signature'. The rename from 'neptune' to 'neptune-signature' is justified to avoid naming conflicts with the existing Neptune ecosystem packages in conda-forge, particularly the main neptune-client
package.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify no naming conflicts exist in conda-forge
curl -s https://conda.anaconda.org/conda-forge/noarch/repodata.json | jq '.packages | keys[]' | grep -i neptune
Length of output: 7111
Line range hint 28-30
: Verify command name after package rename.
The test command still uses neptune
despite the package being renamed to neptune-signature
. Please verify if the command name has changed in the new version.
🧰 Tools
🪛 yamllint
[error] 12-12: syntax error: expected , but found ''
(syntax)
recipes/neptune-signature/meta.yaml
Outdated
number: 0 | ||
script: {{ PYTHON }} -m pip install . --ignore-installed --no-deps -vv | ||
run_exports: | ||
- {{ pin_subpackage('neptune-signature', max_pin="x") }} |
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 YAML syntax error and verify run_exports configuration.
The run_exports
section is correctly configured for ABI compatibility, but there's a YAML syntax error that needs to be fixed.
Apply this diff to fix the indentation:
number: 0
script: {{ PYTHON }} -m pip install . --ignore-installed --no-deps -vv
run_exports:
- - {{ pin_subpackage('neptune-signature', max_pin="x") }}
+ - {{ pin_subpackage('neptune-signature', max_pin='x') }}
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 yamllint
[error] 12-12: syntax error: expected , but found ''
(syntax)
📝 WalkthroughWalkthroughThe pull request introduces significant 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 (
|
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: 1
🧹 Outside diff range and nitpick comments (1)
recipes/neptune-signature/meta.yaml (1)
2-3
: Package rename and major version update require careful migration planningThe package rename from 'neptune' to 'neptune-signature' and the major version bump to 2.0.0 will require:
- Clear communication to users about the package rename
- Migration documentation for existing users
- Deprecation notice in the old package
Consider adding a post-link script to display migration instructions when users try to install the old package name.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
recipes/neptune-signature/meta.yaml
(1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/neptune-signature/meta.yaml
[error] 12-12: syntax error: expected , but found ''
(syntax)
🔇 Additional comments (3)
recipes/neptune-signature/meta.yaml (3)
6-7
: Verify source archive integrity
The source URL and checksum have been updated for version 2.0.0.
✅ Verification successful
Source archive integrity verified successfully
The MD5 checksum (3fcce144d67c74c7921d3b9e36aedacb) of the downloaded archive matches exactly with the one specified in the meta.yaml file.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the source archive checksum
curl -sL https://github.com/phac-nml/neptune/archive/refs/tags/v2.0.0.tar.gz | md5sum
Length of output: 123
Line range hint 28-30
: Verify test command compatibility with v2.0.0
Please ensure that the test command neptune -h
is still valid with the new version, as command-line interfaces often change in major versions.
🧰 Tools
🪛 yamllint
[error] 12-12: syntax error: expected , but found ''
(syntax)
Line range hint 18-26
: Breaking change: Python version requirement significantly restricted
The Python version requirement has been significantly changed from <3
to >=3.10
, which:
- Drops support for Python 2.x (if still needed)
- Drops support for Python 3.0-3.9
- May break compatibility for existing users
Additionally, the removal of the 'drmaa' dependency needs verification.
✅ Verification successful
Based on the repository search results, I can now generate the final response:
Python version change is valid, but drmaa removal is not a concern
The Python version requirement change from <3
to >=3.10
is intentional and valid because:
- Python 2.x support is no longer needed as it's end-of-life
- The package uses modern Python features and dependencies that require Python 3.x
- The dependencies like biopython (1.83) and pandas (2.1.4) require Python >=3.10
Regarding drmaa removal:
- No usage of drmaa was found in the codebase
- The package functionality doesn't appear to depend on it
- Its removal is safe and likely intentional cleanup of unused dependencies
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if drmaa is still used in the codebase
rg -l 'import.*drmaa|from.*drmaa' $(fd -e py -e ipynb . --full-path $(dirname $(find . -name meta.yaml -path '*/neptune-signature/*')))
Length of output: 206
Script:
#!/bin/bash
# Let's first check the git history of the meta.yaml file to understand the dependency changes
git log -p -- recipes/neptune-signature/meta.yaml | grep -A 5 -B 5 "drmaa"
# Also check the entire repository for drmaa usage
rg "drmaa" -l
# Let's also check the Python version change
git log -p -- recipes/neptune-signature/meta.yaml | grep -A 5 -B 5 "python.*3"
Length of output: 311
Script:
#!/bin/bash
# Let's try a different approach to check the current state of the codebase
# First, let's check if there are any Python files in the neptune-signature directory
fd -e py . recipes/neptune-signature/
# Then check the content of these files for any drmaa usage
fd -e py . recipes/neptune-signature/ -x cat {}
# Also check if there's a setup.py or requirements.txt that might give us clues
fd "setup.py|requirements.txt" . recipes/neptune-signature/ -x cat {}
Length of output: 53750
🧰 Tools
🪛 yamllint
[error] 12-12: syntax error: expected , but found ''
(syntax)
recipes/neptune-signature/meta.yaml
Outdated
number: 0 | ||
script: {{ PYTHON }} -m pip install . --ignore-installed --no-deps -vv | ||
run_exports: | ||
- {{ pin_subpackage('neptune-signature', max_pin="x") }} |
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 YAML syntax and verify run_exports configuration
The run_exports section is a good addition for ABI compatibility, but there's a YAML syntax error in this section.
Apply this fix to correct the YAML syntax:
number: 0
script: {{ PYTHON }} -m pip install . --ignore-installed --no-deps -vv
run_exports:
- - {{ pin_subpackage('neptune-signature', max_pin="x") }}
+ - {{ pin_subpackage('neptune-signature', max_pin='x') }}
📝 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.
number: 0 | |
script: {{ PYTHON }} -m pip install . --ignore-installed --no-deps -vv | |
run_exports: | |
- {{ pin_subpackage('neptune-signature', max_pin="x") }} | |
number: 0 | |
script: {{ PYTHON }} -m pip install . --ignore-installed --no-deps -vv | |
run_exports: | |
- {{ pin_subpackage('neptune-signature', max_pin='x') }} |
🧰 Tools
🪛 yamllint
[error] 12-12: syntax error: expected , but found ''
(syntax)
Updates Neptune to v2.0.0. Replaces #40042.
I'm not keen on renaming the tool, but in the time since Neptune's release on Bioconda (8 years ago), someone has released an unrelated tool called Neptune on conda-forge (2 years ago), which can be downloaded by mistake, and the linter tells me I must rename my package.
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>
.