Update pangolin to 4.3.1 #42192
Update pangolin to 4.3.1 #42192
Conversation
|
Leaving this one open to prevent the bot from opening a new one, but see #42191 . |
|
@wm75 I'm not quite sure why we you don't just want merge this - it doesn't matter if there are no code changes, why not have a bioconda package anyways? Some people may want to pull that version as they know it exists. |
|
yeah, saw the pangolin issue so apparently that's a real thing. No strong feelings about this from my side, I merely thought I'd stop propagation of bot PRs upstream (next thing that's going to happen is an autogenerated update PR for Galaxy where we do not want the extra version). |
recipes/pangolin/meta.yaml
Outdated
| - gofasta | ||
| - usher >=0.6.2 | ||
| - usher >=0.5.4 |
There was a problem hiding this comment.
see cov-lineages/pangolin#500 for why usher is pinned to >=0.6.2
|
I tried to consolidate version requirements with pangolin upstream, where version requirements are in setup.py and environment.yml @wm75 suggests that upstream requirements are too lenient - would be good to contribute these to the pangolin repo so that it's not confusing why bioconda is tighter than the pangolin repo specs. |
|
Can this be merged? Are there any updates on it? |
|
Let us merge this if possible and try the fixing of the upstream constraints afterwards. It is not uncommon to have tighter bounds here on bioconda. I believe that this otherwise might never be merged, since this PR is already open for so long. |
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces several updates to the In the requirements section, multiple adjustments are made: the version constraint for 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
recipes/pangolin/meta.yaml(2 hunks)
🧰 Additional context used
🪛 yamllint
recipes/pangolin/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (4)
recipes/pangolin/meta.yaml (4)
1-9: LGTM: Version bump and checksum update look correct.
The version update to 4.3.1 and corresponding SHA256 checksum update are consistent with the package update.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
11-14: LGTM: Build configuration is correct.
Build number reset to 0 is appropriate for a new version.
28-28: Verify the snakemake version constraint change.
The change from <=7.30.1 to !=7.30.1 would allow versions above 7.30.1. Please confirm if this is intentional and if newer versions have been tested for compatibility.
#!/bin/bash
# Check for snakemake version-specific features or known issues
rg -l "snakemake" 22-25: Verify the impact of relaxed version constraints.
Several version constraints have been relaxed:
- biopython: ==1.74 → >=1.74
- scikit-learn: <1.3.0 → >=0.23.1
Please ensure these changes don't introduce compatibility issues.
✅ Verification successful
Let me generate additional verification scripts to check the actual usage of these dependencies in the pangolin codebase.
This will help us understand:
- How these libraries are actually used in the code
- If there are any documented version requirements or compatibility notes
- Whether the code relies on specific features that might be version-dependent
Let me try a different approach to verify the dependency usage.
This will help us understand:
- The complete package configuration
- Any test files that might indicate version requirements
- Build-time modifications that could depend on specific versions
Based on the gathered information, I can now generate the final response:
Version constraint changes appear safe to proceed
The relaxed version constraints are acceptable because:
- No version-specific features or compatibility requirements were found in the package's test files or build scripts
- The minimum versions maintained (biopython >=1.74, scikit-learn >=0.23.1) are stable releases that have been available for a considerable time
- The package is marked as
noarch: python, indicating platform-independent Python code that's less likely to have strict version dependencies
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if these dependencies are used with specific version constraints in the pangolin codebase
rg -l "from sklearn|import sklearn"
rg -l "from Bio|import Bio"
Length of output: 6434
Script:
#!/bin/bash
# Check specific usage of scikit-learn and biopython in pangolin source files
# First, find all Python files in pangolin directory
fd -e py . recipes/pangolin --exec cat {} \; | rg -A 5 "from sklearn|import sklearn|from Bio|import Bio"
# Also check if there are any version-specific features or known compatibility issues mentioned in documentation
fd "README|CHANGELOG" recipes/pangolin --exec cat {} \;
Length of output: 170
Script:
#!/bin/bash
# Check the test requirements and build files for version-specific requirements
cat recipes/pangolin/meta.yaml
# Look for any test files or configuration that might indicate version requirements
fd . recipes/pangolin --type f --exec head -n 50 {} \;
# Check if there are any patches or build scripts that might indicate version dependencies
fd ".*\.(patch|sh)" recipes/pangolin --type f --exec cat {} \;
Length of output: 2747
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
recipes/pangolin/run_test.py (1)
12-13: LGTM: Improved test output visibilityGood enhancement to print the decoded output to stderr before the assertion. This will make test failures more debuggable by showing the actual output when the assertion fails.
Consider adding error handling for the decode operation:
-print(pangolin_proc.stdout.decode(), file=sys.stderr) +try: + decoded_output = pangolin_proc.stdout.decode() + print(decoded_output, file=sys.stderr) +except UnicodeDecodeError as e: + print(f"Failed to decode pangolin output: {e}", file=sys.stderr) + print("Raw output:", pangolin_proc.stdout, file=sys.stderr)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
recipes/pangolin/run_test.py(1 hunks)
🔇 Additional comments (1)
recipes/pangolin/run_test.py (1)
5-5: LGTM: Clean import addition
The sys module import is appropriately placed with other standard library imports.
…d in Snakemake 8)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
recipes/pangolin/meta.yaml (1)
Line range hint
1-58: Consider contributing version constraints upstream.As discussed in the PR comments, there are differences between bioconda's version constraints and upstream specifications. Consider contributing these tighter version bounds back to the pangolin repository's
setup.pyandenvironment.ymlto maintain consistency.🧰 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/pangolin/meta.yaml(1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/pangolin/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (4)
recipes/pangolin/meta.yaml (4)
12-16: LGTM! Build configuration changes look good.
- Build number correctly reset to 0 for new version
- Using
{{ PYTHON }}variable is the preferred approach - Addition of
run_exportswith appropriate pinning helps maintain version compatibility
22-22: LGTM! setuptools addition is appropriate.
Adding setuptools to host requirements is correct as it's needed for the build process.
25-31: Verify dependency version constraints.
The changes to version constraints need verification:
- Relaxing biopython to
>=1.74from exact version - Modified snakemake-minimal constraint to exclude 7.30.1
#!/bin/bash
# Check upstream requirements
echo "Checking pangolin's setup.py and environment.yml for version constraints"
rg -A 5 "biopython|snakemake" 1-9: Verify SHA256 checksum for the new version.
The version bump and source URL look correct. Let's verify the SHA256 checksum matches the source tarball.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
Update
pangolin: 4.3 → 4.3.1recipes/pangolin(click to view/edit other files)@cov-lineagesThis pull request was automatically generated (see docs).