-
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
Add svtopovz python package #51643
Add svtopovz python package #51643
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces a new file, Possibly related PRs
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 (
|
@BiocondaBot please add label |
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 (2)
recipes/svtopovz/meta.yaml (2)
13-21
: Consider relaxing run_exports pin specification.For a visualization tool, the x.x pin specification might be unnecessarily restrictive. Consider using a more relaxed pin unless there are specific API compatibility requirements.
- - {{ pin_subpackage('svtopovz', max_pin="x.x") }} + - {{ pin_subpackage('svtopovz', max_pin="x") }}
34-38
: Enhance test coverage.Current tests only verify command availability. Consider adding basic functionality tests, such as:
- Testing with a small example file
- Verifying output file generation
- Checking common error conditions
Would you like help designing additional test cases for the recipe?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/svtopovz/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/svtopovz/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (2)
recipes/svtopovz/meta.yaml (2)
1-8
: LGTM! Template variables and package metadata are well-structured.The use of Jinja2 templates for version management follows best practices.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
9-12
: Verify source URL structure and package location.The source URL points to
HiFi-SVTopo
but the installation script referencesHiFi-SVTopoVz/
. This mismatch needs verification.#!/bin/bash # Description: Verify the repository structure and installation path # Test 1: Check if the repository contains the expected directory curl -sL https://github.com/PacificBiosciences/HiFi-SVTopo/archive/v0.1.1.tar.gz | tar tz | grep -E "HiFi-SVTopoVz/" # Test 2: Look for setup.py or pyproject.toml to confirm package structure curl -sL https://github.com/PacificBiosciences/HiFi-SVTopo/archive/v0.1.1.tar.gz | tar tz | grep -E "(setup.py|pyproject.toml)$"
about: | ||
home: https://github.com/PacificBiosciences/HiFi-SVTopo | ||
license: "Pacific Biosciences Software License Agreement" | ||
summary: "Complex structural variant visualization for HiFi sequencing data: plotting tool." | ||
dev_url: https://github.com/PacificBiosciences/HiFi-SVTopo | ||
|
||
extra: | ||
recipe-maintainers: | ||
- jbelyeu |
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.
Add license_file specification.
The license section should include a path to the license file to ensure it's properly packaged.
Add license file specification:
about:
home: https://github.com/PacificBiosciences/HiFi-SVTopo
license: "Pacific Biosciences Software License Agreement"
+ license_file: LICENSE
summary: "Complex structural variant visualization for HiFi sequencing data: plotting tool."
dev_url: https://github.com/PacificBiosciences/HiFi-SVTopo
Committable suggestion was skipped due to low confidence.
requirements: | ||
host: | ||
- pip | ||
- python >=3.10 | ||
- cython >=3.0.10 | ||
- setuptools | ||
run: | ||
- python >=3.10 | ||
- matplotlib-base >=3.8.4 | ||
- numpy ==1.26.4 | ||
- scikit-image >=0.24.0 | ||
|
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
🛠️ Refactor suggestion
Consider relaxing numpy version constraint
Based on the verification:
- The package uses standard numpy operations (basic imports as
np
) without any indication of version-specific features - No pandas dependency is used or needed in the codebase
- The exact numpy pin (==1.26.4) in meta.yaml matches requirements.txt but could be relaxed to a minimum version constraint for better compatibility
- - numpy ==1.26.4
+ - numpy >=1.26.4
🔗 Analysis chain
Review dependency specifications.
- The exact pin on numpy (==1.26.4) might cause dependency conflicts in the conda environment. Consider using a minimum version constraint instead.
- Consider if pandas is needed as it's commonly used with numpy/matplotlib for genomic data visualization.
Consider updating numpy dependency:
- - numpy ==1.26.4
+ - numpy >=1.26.4
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the package uses pandas
# Look for pandas imports in the source code
curl -sL https://github.com/PacificBiosciences/HiFi-SVTopo/archive/v0.1.1.tar.gz | tar tz | grep -E "\.py$" | while read -r file; do
echo "Checking $file for pandas imports..."
curl -sL https://github.com/PacificBiosciences/HiFi-SVTopo/archive/v0.1.1.tar.gz | tar xz -O "*/$file" | grep -E "import pandas|from pandas"
done
Length of output: 6103
Script:
#!/bin/bash
# Let's try a different approach to check for pandas usage
# First, let's check the repository content structure
curl -sL https://github.com/PacificBiosciences/HiFi-SVTopo/archive/v0.1.1.tar.gz | tar tz
# Then let's examine setup.py or pyproject.toml for dependencies
curl -sL https://github.com/PacificBiosciences/HiFi-SVTopo/archive/v0.1.1.tar.gz | tar xz --wildcards "*/setup.py" -O 2>/dev/null || true
curl -sL https://github.com/PacificBiosciences/HiFi-SVTopo/archive/v0.1.1.tar.gz | tar xz --wildcards "*/pyproject.toml" -O 2>/dev/null || true
# Let's also check the source files for pandas usage
curl -sL https://github.com/PacificBiosciences/HiFi-SVTopo/archive/v0.1.1.tar.gz | tar xz --wildcards "*.py" --to-stdout 2>/dev/null | grep -E "import pandas|from pandas" || true
Length of output: 6764
Script:
#!/bin/bash
# Let's check the requirements.txt file to see the actual dependencies
curl -sL https://github.com/PacificBiosciences/HiFi-SVTopo/archive/v0.1.1.tar.gz | tar xz --wildcards "*/requirements.txt" -O 2>/dev/null || true
# Also check the actual usage of numpy to understand if exact pinning is necessary
curl -sL https://github.com/PacificBiosciences/HiFi-SVTopo/archive/v0.1.1.tar.gz | tar xz --wildcards "*.py" --to-stdout 2>/dev/null | grep -E "import numpy|from numpy" || true
Length of output: 449
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.
Just added a few finishing touches, the rest LGTM @jbelyeu! Thanks!
This is a new python tool for visualization of complex genomic structural variants from HiFi sequencing data. Initial release.
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>
.