-
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 pyopenms to 3.2.0 #51014
Update pyopenms to 3.2.0 #51014
Conversation
📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe pull request updates 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: 0
🧹 Outside diff range and nitpick comments (3)
recipes/pyopenms/meta.yaml (3)
14-14
: Source URL updated, but consider a more robust solution.The source URL has been updated to point to the correct tarball for version 3.2.0. However, the hardcoded URL and the omission of the trailing zero in the version number could lead to issues in future updates.
Consider implementing a more robust solution that doesn't require hardcoding the URL or omitting parts of the version number. This could involve:
- Updating the GitHub Actions workflow to maintain consistency in release asset naming.
- Implementing a Jinja2 template that can handle version number variations.
Example:
{% set version_parts = version.split('.') %} {% set release_version = '.'.join(version_parts[:2]) if version_parts[2] == '0' else version %} url: https://github.com/OpenMS/OpenMS/releases/download/release%2F{{ version }}/OpenMS-{{ release_version }}.tar.gzThis approach would handle both "x.y.0" and "x.y.z" version formats automatically.
Line range hint
46-56
: Address the run_exports issue.There's a comment indicating that run_exports in libopenms is not working as expected due to bugs, leading to the duplication of library dependencies in the run section.
Consider the following actions:
- Investigate and fix the run_exports issue in the libopenms package.
- Once fixed, remove the duplicated dependencies from the run section of this recipe.
- If the issue persists, open an issue in the appropriate repository (likely the conda-forge feedstock for libopenms) to track and resolve the run_exports problem.
This will help maintain a cleaner and more maintainable recipe in the long run.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
Line range hint
57-61
: Enhance the test section.The current test section only imports the package and prints its version. While this is a good basic test, it could be more comprehensive to ensure the package is functioning correctly.
Consider adding more tests to verify the functionality of pyopenms. For example:
- Test creating and manipulating some basic OpenMS objects.
- Verify that key functions or methods are available and working as expected.
- If possible, include a small dataset and perform a simple analysis task.
Example of an enhanced test section:
test: imports: - pyopenms commands: - python -c "import pyopenms; print(pyopenms.__version__)" - python -c "from pyopenms import *; exp = MSExperiment(); print(exp)" - python -c "from pyopenms import *; spec = MSSpectrum(); spec.set_mz([100, 200]); spec.set_intensities([1000, 2000]); print(spec.get_peaks())" requires: - pip pip_check: trueThis will provide more confidence in the package's functionality after installation.
🧰 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/pyopenms/meta.yaml (2 hunks)
🔇 Additional comments (2)
recipes/pyopenms/meta.yaml (2)
2-2
: Version update looks good.The package version has been correctly updated to 3.2.0, which aligns with the PR objective of updating pyopenms to version 3.2.0.
15-15
: MD5 checksum updated, verify correctness.The MD5 checksum has been updated to match the new source tarball for version 3.2.0.
To ensure the integrity of the package, please verify that the provided MD5 checksum is correct for the new source tarball. You can use the following script to download the tarball and verify its checksum:
If the calculated checksum matches the expected one, the update is correct.
✅ Verification successful
MD5 checksum verification successful.
The calculated MD5 checksum matches the expected value, ensuring the integrity of the source tarball for version 3.2.0.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Download the source tarball and verify its MD5 checksum # Download the tarball curl -L -o OpenMS-3.2.tar.gz https://github.com/OpenMS/OpenMS/releases/download/release%2F3.2.0/OpenMS-3.2.tar.gz # Calculate and display the MD5 checksum echo "Calculated MD5 checksum:" md5sum OpenMS-3.2.tar.gz echo "Expected MD5 checksum:" echo "60ebfe8a45f7e1fb5566d21231b460e8" # Clean up rm OpenMS-3.2.tar.gzLength of output: 1196
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/pyopenms/meta.yaml (1)
56-56
: LGTM: Eigen added to run requirements. Consider future optimization.The addition of "eigen >=3.3" to the run requirements is consistent with the build and host requirements. This redundancy appears necessary due to the mentioned issues with run_exports in libopenms.
For future maintenance:
- Monitor the status of the run_exports functionality in libopenms.
- If the run_exports issues are resolved, consider simplifying this recipe by removing redundant dependencies from the run section.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/pyopenms/meta.yaml (5 hunks)
🔇 Additional comments (4)
recipes/pyopenms/meta.yaml (4)
2-2
: LGTM: Version update is correct.The version has been correctly updated to 3.2.0, which aligns with the PR objective.
29-29
: LGTM: Eigen added to build requirements.The addition of "eigen >=3.3" to the build requirements is appropriate and consistent with the changes mentioned in the summary.
38-38
: LGTM: Eigen added to host requirements.The addition of "eigen >=3.3" to the host requirements is appropriate and consistent with the build requirements.
14-15
: Verify the source URL and checksum.The source URL and MD5 checksum have been updated for the new version. However, there are a few points to consider:
- The URL structure has changed from "Release3.1.0" to "release%2F3.2.0".
- The tarball name is now "OpenMS-3.2.tar.gz" instead of "OpenMS-3.2.0-src.tar.gz".
- There's a comment about hardcoding the URL due to discarding the trailing 0 during GitHub Actions.
Please ensure that:
- The URL is correct and accessible.
- The MD5 checksum matches the downloaded tarball.
- The discrepancy in version number (3.2 vs 3.2.0) in the URL is intentional and doesn't cause issues.
To verify the URL and checksum, you can run the following script:
Compare the output MD5 sum with the one specified in the recipe (60ebfe8a45f7e1fb5566d21231b460e8).
✅ Verification successful
Verified: The source URL is accessible (HTTP 302) and the MD5 checksum matches the specified value. No issues detected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the source URL and checksum for pyopenms 3.2.0 # Test: Check if the URL is accessible curl -sI https://github.com/OpenMS/OpenMS/releases/download/release%2F3.2.0/OpenMS-3.2.tar.gz | head -n 1 # Test: Download the tarball and verify its MD5 checksum curl -sL https://github.com/OpenMS/OpenMS/releases/download/release%2F3.2.0/OpenMS-3.2.tar.gz | md5sumLength of output: 261
is eigen only 3.2? https://anaconda.org/bioconda/eigen |
Log says eigen==3.4.0 during build, run and host time.. |
Not sure, but I think they switched to bump version on Conda-forge: https://anaconda.org/conda-forge/eigen (and that's what is used during build time here) |
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 (2)
recipes/pyopenms/meta.yaml (2)
14-15
: LGTM: Updated source URL and MD5 checksumThe source URL and MD5 checksum have been correctly updated for version 3.2.0.
Consider adding a TODO comment to remind about fixing the URL hardcoding in the next release, as mentioned in the inline comment.
Line range hint
1-85
: Summary: pyopenms update to 3.2.0 looks good overallThe changes in this PR successfully update the pyopenms package to version 3.2.0 and address the eigen dependency concerns raised in the PR comments. Here's a summary of the key points:
- Version updated to 3.2.0
- Source URL and MD5 checksum updated correctly
- eigen >=3.3.2 added to build, host, and run requirements
To ensure a smooth integration of these changes, please consider the following recommendations:
- Run the suggested verification scripts to confirm eigen version compatibility and check for potential conflicts with downstream dependencies.
- Add a TODO comment near the hardcoded URL to remind about fixing it in the next release.
- After merging, monitor the build and test processes closely to catch any unforeseen issues that may arise from these changes.
If all verifications pass and no conflicts are found, this PR is ready to be merged.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/pyopenms/meta.yaml (5 hunks)
🧰 Additional context used
🔇 Additional comments (3)
recipes/pyopenms/meta.yaml (3)
2-2
: LGTM: Version update to 3.2.0The version update is consistent with the PR objective to update pyopenms to version 3.2.0.
38-38
: LGTM: Added eigen to host and run requirementsThe addition of
eigen >=3.3.2
to both host and run requirements ensures consistency across build, host, and runtime environments.To ensure this change doesn't negatively impact downstream dependencies, please run the following script:
#!/bin/bash # Description: Check for potential conflicts with downstream dependencies # Expected result: No conflicts or incompatibilities found # Search for recipes that depend on pyopenms dependent_recipes=$(grep -R "pyopenms" recipes | grep "requirements" | cut -d: -f1 | sort -u) # Check if any of these recipes also specify a different version of eigen for recipe in $dependent_recipes; do eigen_version=$(grep "eigen" "$recipe" | grep -v "pyopenms") if [ ! -z "$eigen_version" ]; then echo "Potential conflict in $recipe:" echo "$eigen_version" fi done # If no output is produced, it means no potential conflicts were foundAlso applies to: 56-56
29-29
: LGTM: Added eigen to build requirementsThe addition of
eigen >=3.3.2
to the build requirements is consistent with the discussion in the PR comments.To ensure this change is sufficient, please run the following script to check if eigen 3.4.0 (mentioned in the build log) is compatible with this requirement:
✅ Verification successful
Eigen build requirement verified successfully
The specified
eigen >=3.3.2
is present in the following files:
recipes/pyopenms/meta.yaml
recipes/openms-meta/meta.yaml
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify eigen version compatibility # Expected result: Should return a version number >= 3.3.2 # Check if conda-forge has eigen 3.4.0 or later conda search -c conda-forge eigen | grep -E "3\.[4-9]\.[0-9]|[4-9]\.[0-9]\.[0-9]" # If the above command returns results, it confirms that conda-forge has a compatible versionLength of output: 95
Script:
#!/bin/bash # Description: Verify available eigen versions on conda-forge # Expected result: List of eigen versions >= 3.3.2 conda search -c conda-forge eigen | awk '$2 ~ /^3\.(3|[4-9])\./ {print $2}'Length of output: 150
Script:
#!/bin/bash # Description: Check for eigen dependencies in environment files # Expected result: Lines mentioning eigen with version >= 3.3.2 fd "eigen" . --type f \( -name "environment.yml" -o -name "*.yaml" -o -name "*.yml" \) --exec grep -E "eigen\s*>=\s*3\.3\.2" {}Length of output: 360
Script:
#!/bin/bash # Description: Search for eigen version specifications in YAML files # Expected result: Lines containing 'eigen' with version >=3.3.2 rg -i 'eigen\s*>=\s*3\.3\.2' --type=yamlLength of output: 345
Script:
#!/bin/bash # Description: Use ast-grep to parse YAML files for eigen dependency and verify version # Expected result: Matches where eigen is required with version >=3.3.2 ast-grep --lang yaml --pattern 'dependencies: *\n - eigen >=3.3.2' .Length of output: 528
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 (2)
recipes/pyopenms/meta.yaml (2)
14-14
: LGTM: Source URL updated, but consider future improvementsThe source URL has been correctly updated to reflect the new version. However, the comment suggests that this URL is hardcoded due to discarding the trailing 0 during GitHub Actions.
Consider creating a follow-up task to implement a more dynamic URL generation in the next release to avoid hardcoding.
56-56
: LGTM: eigen dependency added to run section, but investigate run_exportsThe eigen dependency has been consistently added to the run section with version 3.3.4, matching the build and host sections.
The comment suggests that run_exports in libopenms is not working as expected. Consider investigating this issue to potentially simplify dependency management in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/pyopenms/meta.yaml (5 hunks)
🧰 Additional context used
🔇 Additional comments (3)
recipes/pyopenms/meta.yaml (3)
2-2
: LGTM: Version update to 3.2.0The version update from 3.1.0 to 3.2.0 is consistent with the PR objective.
29-29
:⚠️ Potential issueClarify eigen version discrepancy
The eigen dependency has been added with version 3.3.4 in both the build and host sections. However, this version differs from the 3.4.0 version mentioned in the PR comments.
Please clarify the correct version of eigen to use. If 3.3.4 is correct, update the PR comments. If 3.4.0 is correct, update the meta.yaml file.
To verify the available eigen versions, run:
#!/bin/bash # Description: Check available eigen versions in conda-forge conda search -c conda-forge eigenAlso applies to: 38-38
15-15
: LGTM: MD5 checksum updatedThe MD5 checksum has been updated for the new version, which is necessary. However, it's important to ensure that this checksum is correct.
Please run the following script to verify the MD5 checksum:
✅ Verification successful
MD5 checksum verified successfully
The calculated MD5 checksum matches the expected value.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the MD5 checksum of the downloaded source file # Download the source file curl -L -o OpenMS-3.2.tar.gz https://github.com/OpenMS/OpenMS/releases/download/release%2F3.2.0/OpenMS-3.2.tar.gz # Calculate and compare MD5 checksum calculated_md5=$(md5sum OpenMS-3.2.tar.gz | awk '{print $1}') expected_md5="e15b34a4ccaf7070d186fd01a6d57f3e" if [ "$calculated_md5" = "$expected_md5" ]; then echo "MD5 checksum verified successfully" else echo "MD5 checksum verification failed" echo "Calculated: $calculated_md5" echo "Expected: $expected_md5" fi # Clean up rm OpenMS-3.2.tar.gzLength of output: 1421
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
🧹 Outside diff range and nitpick comments (1)
recipes/pyopenms/conda_build_config.yaml (1)
Line range hint
1-19
: Overall positive impact, consider updating documentation.The changes to both Python and numpy versions are consistent and provide a good balance between supporting newer setups and maintaining backward compatibility. This approach allows the package to cater to a wider range of user environments.
Consider updating the package's documentation to reflect these changes:
- Mention support for Python 3.12.* in the compatibility section.
- Update the supported numpy versions, highlighting the addition of 1.26.* support.
- If there are any known issues or considerations for users upgrading to these newer versions, include them in the documentation.
This will help users understand the expanded compatibility and make informed decisions when using or upgrading the package.
- 1.23.* | ||
- 1.26.* |
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.
💡 Codebase verification
Attention: Build Failures Detected with numpy 1.26.*
Several build failures have been identified related to the updated numpy 1.26.* versions:
- Unsatisfiable dependencies in
recipes/gemini/build_failure.osx-64.yaml
related topython_abi
. - Overdependency warnings in multiple recipes such as
recipes/pydmtools
andrecipes/pybmtools
indicating potential issues with numpy 1.26.4 usage.
Please investigate and resolve these build failures to ensure compatibility with numpy 1.26.* across all affected packages.
🔗 Analysis chain
Approved: Updated numpy versions for better compatibility.
The update to include numpy 1.26.* while removing one instance of 1.23.* is a good step towards supporting newer numpy versions. This change can improve compatibility with newer Python versions and other dependencies.
Please ensure that the package has been tested and is fully compatible with numpy 1.26.*. You can verify this by running the following command:
Additionally, verify that this change doesn't break compatibility with existing setups that may still rely on numpy 1.23.*.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if there are any test results or build logs for numpy 1.26.*
# Test: Search for numpy 1.26 in test results or build logs. Expect: Occurrences of numpy 1.26 in test or build context.
rg --type yaml -i "numpy.*1\.26|1\.26.*numpy" && echo "numpy 1.26 found in configuration" || echo "No explicit mention of numpy 1.26 in configuration"
Length of output: 6237
@@ -2,14 +2,16 @@ python: | |||
- 3.9.* | |||
- 3.10.* | |||
- 3.11.* | |||
- 3.12.* |
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.
💡 Codebase verification
Potential Compatibility Issues with Python 3.12. Integration.*
While adding Python 3.12.* support to recipes/pyopenms/conda_build_config.yaml
is a positive step towards keeping the package up-to-date, the current configuration reveals several compatibility conflicts:
-
Conflicting Python Version Requirements:
- Many recipes specify Python versions with upper limits below 3.12, such as
<3.12
, which may lead to conflicts when Python 3.12.* is introduced.
- Many recipes specify Python versions with upper limits below 3.12, such as
-
Build Failures:
- Multiple build failures indicate that dependencies or packages are not yet compatible with Python 3.12.*, requiring attention before full support can be confirmed.
Recommendation:
Please address the identified compatibility issues across the codebase to ensure that all dependent packages can support Python 3.12.* seamlessly.
🔗 Analysis chain
Excellent addition of Python 3.12. support.*
Adding support for Python 3.12.* is a positive change that keeps the package up-to-date with the latest Python release. This allows users with newer Python installations to use the package.
Please ensure that the package has been tested and is fully compatible with Python 3.12. You can verify this by running the following command:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if there are any test results or build logs for Python 3.12
# Test: Search for Python 3.12 in test results or build logs. Expect: Occurrences of Python 3.12 in test or build context.
rg --type yaml -i "python.*3\.12|3\.12.*python" && echo "Python 3.12 found in configuration" || echo "No explicit mention of Python 3.12 in configuration"
Length of output: 88540
@BiocondaBot please update |
I encountered an error updating your PR branch. You can report this to bioconda/core if you'd like. |
@bioconda/core can someone include this one in a bulk-PR? |
Reposting for @jonasscheid to enable pings (courtesy of the BiocondaBot):
|
@jonasscheid I have a working recipe that builds in #51753 |
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>
.