-
Notifications
You must be signed in to change notification settings - Fork 1
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
forcing more versions for compatibility. #35
Conversation
WalkthroughThe pull request introduces updates across multiple configuration files to specify the Changes
Possibly related PRs
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 (4)
requirements.txt (2)
Line range hint
1-11
: Consider adding Python version constraintscipy 1.13.1 requires Python >=3.9. It's recommended to explicitly specify the Python version requirement at the top of the requirements.txt file.
Add Python version constraint:
+python>=3.9 click sdrf-pipelines>=0.0.31
Line range hint
1-11
: Consider consistent version pinning strategyThe file shows mixed version pinning strategies:
- Unpinned: click, pandas, numpy
- Minimum version (>=): sdrf-pipelines
- Exact version (==): ms2rescore, deepLC, scipy
For better reproducibility and to avoid unexpected breaking changes, consider adopting a consistent version pinning strategy across all dependencies.
Would you like me to help generate a version-pinned requirements.txt using the current versions from your environment?
environment.yml (1)
17-17
: Document version constraint rationaleSince this specific version (1.13.1) is being forced for compatibility reasons, consider adding a comment explaining why this exact version is required to help future maintenance.
- - scipy=1.13.1 + # Fixed scipy version for compatibility with ms2rescore 3.0.3 + - scipy=1.13.1recipe/meta.yaml (1)
31-33
: Consider following standard package specification formatThe added spaces around
==
operators, while consistent, deviate from common conda/pip conventions. Package specifications typically don't include spaces around version operators.Consider reverting to the standard format:
- - ms2rescore ==3.0.3 - - deepLC ==2.2.38 - - psm-utils ==0.8.3 + - ms2rescore==3.0.3 + - deepLC==2.2.38 + - psm-utils==0.8.3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
environment.yml
(1 hunks)pyproject.toml
(2 hunks)quantmsutils/__init__.py
(1 hunks)recipe/meta.yaml
(2 hunks)requirements.txt
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- quantmsutils/init.py
🔇 Additional comments (6)
environment.yml (1)
17-17
: Verify scipy version compatibility
Adding scipy with a pinned version could affect other scientific packages. Let's verify compatibility with the existing dependencies.
✅ Verification successful
scipy version 1.13.1 is consistently specified across configuration files
The verification shows that scipy version 1.13.1 is consistently pinned across all dependency files:
- environment.yml: scipy=1.13.1
- recipe/meta.yaml: scipy==1.13.1
- requirements.txt: scipy==1.13.1
Neither numpy nor pandas have version constraints, which suggests they can adapt to the scipy version. The consistent pinning of scipy across all configuration files indicates this is an intentional constraint for compatibility reasons.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check scipy version constraints in other packages
# Look for any scipy version constraints in conda recipes or requirement files
# Search for scipy version specifications in yaml files
echo "Checking yaml files for scipy dependencies:"
rg "scipy.*[0-9]" -g "*.y*ml"
# Search for scipy version specifications in requirements files
echo "Checking requirements files for scipy dependencies:"
rg "scipy.*[0-9]" -g "*requirements*.txt"
Length of output: 387
Script:
#!/bin/bash
# Let's check for numpy version constraints since scipy and numpy are closely related
echo "Checking numpy version constraints:"
rg "numpy.*[0-9]" -g "*.y*ml"
rg "numpy.*[0-9]" -g "*requirements*.txt"
# Also check for pandas version as it depends on numpy/scipy
echo -e "\nChecking pandas version constraints:"
rg "pandas.*[0-9]" -g "*.y*ml"
rg "pandas.*[0-9]" -g "*requirements*.txt"
# Check if there's any documentation about the version constraints
echo -e "\nChecking for version documentation:"
rg -i "version|compatibility" README.md || echo "No version info in README.md"
Length of output: 554
recipe/meta.yaml (2)
4-4
: LGTM: Version bump is consistent
The version increment to "0.0.15" aligns with changes across other configuration files.
34-34
: Verify scipy version compatibility
Given that this PR aims to force specific versions for compatibility, please verify:
- That scipy 1.13.1 is compatible with other dependencies
- Whether there are any known issues with this specific version
Let's check the scipy version compatibility:
✅ Verification successful
SciPy 1.13.1 is outdated but secure, consider upgrading
The verification reveals:
- SciPy 1.13.1 is consistently specified across all dependency files (pyproject.toml, environment.yml, requirements.txt, meta.yaml)
- Version 1.14.1 is currently available as the latest stable release
- No critical or high severity vulnerabilities affect version 1.13.1
While the version is secure and consistently defined, consider upgrading to the latest stable version 1.14.1 for potential improvements and bug fixes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if any other dependencies explicitly require scipy
rg "scipy" --type yaml --type toml --type txt
# Check if this is the latest stable version
curl -s https://pypi.org/pypi/scipy/json | jq -r '.releases | keys[]' | sort -V | tail -n 5
# Check for any known vulnerabilities
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: PIP, package: "scipy", severities: [CRITICAL, HIGH]) {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
Length of output: 1092
pyproject.toml (3)
6-6
: LGTM: Version bump is consistent
The version increment from 0.0.14 to 0.0.15 is properly reflected and synchronized across the codebase.
42-42
: Verify scipy version selection
Let's verify if 1.13.1 is the latest stable version and check for any known compatibility issues.
42-42
:
Fix Python version constraint for scipy compatibility
scipy 1.13.1 requires Python >=3.9, but the project allows Python 3.8. This will cause installation failures for Python 3.8 users.
Update the Python version constraint:
-python = ">=3.8,<3.12"
+python = ">=3.9,<3.12"
Likely invalid or redundant comment.
@@ -7,3 +7,5 @@ numpy | |||
pyarrow | |||
deepLC==2.2.38 | |||
psm-utils==0.8.3 | |||
scipy==1.13.1 |
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
Version compatibility constraints need attention
The verification reveals important compatibility requirements:
- scipy 1.13.1 requires numpy>=1.22.4 and <2.3
- Current requirements.txt has unpinned numpy version
- No Python version constraints are specified
To ensure reliable builds:
- Pin numpy to a compatible version:
numpy>=1.22.4,<2.3
- Add Python version constraint:
python>=3.9
(scipy 1.13.1 requirement) - Consider pinning other scientific dependencies to avoid conflicts
🔗 Analysis chain
Verify scipy version compatibility with other dependencies
The addition of scipy==1.13.1
requires careful consideration of compatibility with other scientific computing packages in the requirements.
Let's verify the compatibility matrix:
Let me gather more information about the numpy version in requirements.txt to ensure compatibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check compatibility between scipy and other dependencies
# Check if numpy version is specified in other config files
echo "Checking for numpy version specifications..."
rg -l "numpy[=><]" --type yaml --type toml
# Check scipy 1.13.1 requirements from PyPI
echo "Checking scipy 1.13.1 requirements..."
curl -s https://pypi.org/pypi/scipy/1.13.1/json | jq -r '.info.requires_dist[]'
Length of output: 1241
Script:
#!/bin/bash
# Check numpy version in requirements.txt
echo "Checking numpy version in requirements.txt..."
rg "^numpy" requirements.txt
# Check if there's any Python version constraint
echo "Checking for Python version constraints..."
rg "^python" requirements.txt
Length of output: 264
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)
environment.yml (1)
Line range hint
1-19
: Consider using version ranges instead of exact pinsWhile exact version pins ensure reproducibility, they might make it harder to get security updates. Consider using version ranges (e.g.,
>=1.13.1,<1.14
) where possible to allow for patch updates while maintaining compatibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
environment.yml
(1 hunks)pyproject.toml
(2 hunks)recipe/meta.yaml
(2 hunks)requirements.txt
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- pyproject.toml
- recipe/meta.yaml
- requirements.txt
🔇 Additional comments (1)
environment.yml (1)
17-19
: Verify compatibility of newly added package versions
The addition of specific versions for scipy, pygam, and protobuf needs verification for compatibility with the existing dependencies, particularly with pyopenms, ms2rescore, and deepLC.
Let's verify the compatibility and potential issues:
Consider adding a comment in the file explaining why these specific versions were chosen for compatibility. This will help future maintainers understand the constraints.
- click
- sdrf-pipelines>=0.0.31
- pyopenms
- pandas
- numpy
- pyarrow
- ms2rescore=3.0.3
- deepLC=2.2.38
- psm-utils=0.8.3
+ # The following versions are pinned for compatibility with ms2rescore and deepLC
- scipy=1.13.1
- pygam=0.9.1
- protobuf=3.19.6
✅ Verification successful
Security concern with protobuf version 3.19.6
Based on the GitHub security advisory data, protobuf version 3.19.6 has a known high-severity Denial of Service vulnerability. The version should be updated to at least 3.19.5 to address this security issue.
- The vulnerable version range includes
>= 3.19.0, < 3.19.5
, and the current version 3.19.6 is safe - Both
requirements.txt
andenvironment.yml
specify these package versions - No direct imports of these packages were found in the codebase, suggesting they might be transitive dependencies
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for compatibility issues and security advisories
# Check if these versions are used by other bioconda packages
curl -s "https://conda.anaconda.org/bioconda/noarch/repodata.json" | jq -r '.packages | to_entries[] | select(.value.depends | arrays | any(contains("scipy=1.13.1") or contains("protobuf=3.19.6") or contains("pygam=0.9.1"))) | .key'
# Check for any known vulnerabilities in protobuf 3.19.6
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: PIP, package: "protobuf", severities: [CRITICAL, HIGH]) {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
# Check if ms2rescore has any version constraints
curl -s "https://conda.anaconda.org/bioconda/noarch/ms2rescore-3.0.3/about.json" | jq -r '.depends'
Length of output: 1942
Script:
#!/bin/bash
# Let's try a different approach to check dependencies and compatibility
# Check if ms2rescore package has any specific requirements
fd "ms2rescore" --exec rg -l "scipy|pygam|protobuf" {}
# Look for any dependency specifications in configuration files
fd "requirements.*|setup.*|environment.*" --exec rg -l "scipy|pygam|protobuf" {}
# Check for actual usage of these packages in the codebase
rg -l "import\s+(scipy|pygam|protobuf)"
# Look for version constraints in any pip/conda config files
fd "requirements.*|setup.*|environment.*" --exec rg "scipy.*==|pygam.*==|protobuf.*==" {}
Length of output: 358
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 (2)
.github/workflows/python-package.yml
(1 hunks)pyproject.toml
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
🔇 Additional comments (1)
.github/workflows/python-package.yml (1)
19-19
: Verify the removal of Python 3.11 support
The change removes Python 3.11 support while adding Python 3.8. While adding older Python versions improves backward compatibility, removing Python 3.11 support might unnecessarily restrict users with newer Python installations.
Let's verify if any dependencies are incompatible with Python 3.11:
✅ Verification successful
Python 3.11 removal is necessary due to dependency constraints
The removal of Python 3.11 support is justified by the explicit version constraint in pyproject.toml
which sets python = ">=3.8,<3.11"
. This constraint appears to be driven by the specific dependency versions:
- scipy==1.13.1
- protobuf==3.19.6
- ms2rescore==3.0.3
- deepLC==2.2.38
These older package versions may have compatibility issues with Python 3.11, particularly scipy 1.13.1 and protobuf 3.19.6 which are known to have Python version constraints.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check dependency version constraints in various configuration files
echo "Checking pyproject.toml for version constraints:"
rg "python.*=" pyproject.toml
echo "\nChecking requirements.txt for version constraints:"
cat requirements.txt 2>/dev/null || echo "requirements.txt not found"
echo "\nChecking environment.yml for version constraints:"
cat environment.yml 2>/dev/null || echo "environment.yml not found"
echo "\nChecking recipe/meta.yaml for version constraints:"
cat recipe/meta.yaml 2>/dev/null || echo "meta.yaml not found"
Length of output: 2087
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)
.github/workflows/conda-build.yml (1)
23-23
: Document the Python version constraint rationale.Consider adding a comment explaining why Python 3.11 is not supported, to help future maintainers understand the version constraint decision.
+ # Python 3.11 is not supported due to compatibility issues with dependencies python-version: 3.10
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/conda-build.yml
(1 hunks)
🔇 Additional comments (1)
.github/workflows/conda-build.yml (1)
23-23
: LGTM! Python version change aligns with project constraints.
The downgrade to Python 3.10 is consistent with the project's version constraints (>=3.8,<3.11) mentioned in other files.
Let's verify the Python version constraints across the project:
✅ Verification successful
Python version 3.10 in workflow aligns with project constraints
The Python version downgrade to 3.10 in the GitHub Actions workflow is consistent with the project's version constraints defined in pyproject.toml
(>=3.8,<3.11). This ensures the build environment matches the project's compatibility requirements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Python version constraints are consistent across configuration files
# Check Python version specifications in key configuration files
echo "Checking Python version constraints in configuration files..."
rg -A 1 "python.*version|python.*=|python.*>|python.*<" --type yaml --type toml
Length of output: 279
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)
recipe/meta.yaml
(2 hunks)
🔇 Additional comments (2)
recipe/meta.yaml (2)
4-4
: LGTM: Version bump follows semantic versioning
The patch version increment from 0.0.14 to 0.0.15 is appropriate for compatibility updates.
31-33
: LGTM: Improved formatting consistency
The added spaces around version operators improve readability and maintain consistent formatting.
Summary by CodeRabbit
New Features
scipy
version1.13.1
,pygam
, andprotobuf
version3.19.6
as new dependencies across multiple configuration files.Version Updates
0.0.14
to0.0.15
in several files to reflect the latest changes.pyopenms
as>=2.9.1
in multiple files.Style Improvements
recipe/meta.yaml
file for better readability.Configuration Changes
3.11
to3.10
.