-
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 pykofamsearch
#51537
Add pykofamsearch
#51537
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces a new file, 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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/pykofamsearch/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/pykofamsearch/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (5)
recipes/pykofamsearch/meta.yaml (5)
1-6
: LGTM: Package metadata is correctly defined.The package name and version are properly set using Jinja2 templating, which is a standard practice in Conda recipes.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
43-49
: LGTM: About section is complete and accurate.All required metadata fields are present, including home page, summary, license information, and development URL. The license file is also correctly specified.
1-1
: Note on static analysis warningThe yamllint tool reports a syntax error for the '%' character at the beginning of the file. This is a false positive and can be safely ignored. The '%' character is part of the Jinja2 templating syntax, which is valid and commonly used in Conda recipe YAML files.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
25-33
: LGTM: Requirements are well-defined.The host and run requirements are appropriately specified for a Python package.
To ensure the accuracy of the run requirements, please verify them against the package's
setup.py
orpyproject.toml
file. You can use the following command to check the package's metadata:#!/bin/bash # Fetch and display the package's metadata curl -sL https://pypi.org/pypi/pykofamsearch/json | jq '.info.requires_dist'Compare the output with the run requirements in the recipe and adjust if necessary.
8-10
: LGTM: Source information is correctly specified.The source URL is properly formatted for PyPI packages, and a SHA256 checksum is provided for integrity verification.
To ensure the integrity of the package, please verify the SHA256 checksum independently. You can use the following command:
Compare the output with the checksum in the recipe.
recipes/pykofamsearch/meta.yaml
Outdated
test: | ||
commands: | ||
- pykofamsearch --help | ||
- reformat_enzymes --help | ||
- reformat_pykofamsearch --help | ||
- serialize_kofam_models --help | ||
- subset_serialized_models --helps |
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.
Test commands look good, but there's a typo in the last command.
The approach of testing the help output for each entry point is a good basic test.
There's a typo in the last test command:
- - subset_serialized_models --helps
+ - subset_serialized_models --help
Please correct this to ensure all tests pass.
📝 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.
test: | |
commands: | |
- pykofamsearch --help | |
- reformat_enzymes --help | |
- reformat_pykofamsearch --help | |
- serialize_kofam_models --help | |
- subset_serialized_models --helps | |
test: | |
commands: | |
- pykofamsearch --help | |
- reformat_enzymes --help | |
- reformat_pykofamsearch --help | |
- serialize_kofam_models --help | |
- subset_serialized_models --help |
recipes/pykofamsearch/meta.yaml
Outdated
build: | ||
script: {{ PYTHON }} -m pip install . -vvv --no-deps --no-build-isolation --no-cache-dir | ||
number: 0 | ||
noarch: python | ||
entry_points: | ||
- pykofamsearch=pykofamsearch.pykofamsearch:main | ||
- reformat_enzymes=pykofamsearch.reformat_enzymes:main | ||
- reformat_pykofamsearch=pykofamsearch.reformat_pykofamsearch:main | ||
- serialize_kofam_models=pykofamsearch.serialize_kofam_models:main | ||
- subset_serialized_models=pykofamsearch.subset_serialized_models:main | ||
run_exports: | ||
- {{ pin_subpackage("pyhmmsearch", max_pin=None) }} |
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.
Build section looks good, but there's an issue in the run_exports
section.
The build script, build number, noarch
specification, and entry points are all correctly defined. However, there's an issue in the run_exports
section:
The run_exports
section references "pyhmmsearch" instead of "pykofamsearch". This should be corrected:
run_exports:
- - {{ pin_subpackage("pyhmmsearch", max_pin=None) }}
+ - {{ pin_subpackage("pykofamsearch", max_pin=None) }}
Also, consider if max_pin=None
is the desired pinning strategy for this package.
📝 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.
build: | |
script: {{ PYTHON }} -m pip install . -vvv --no-deps --no-build-isolation --no-cache-dir | |
number: 0 | |
noarch: python | |
entry_points: | |
- pykofamsearch=pykofamsearch.pykofamsearch:main | |
- reformat_enzymes=pykofamsearch.reformat_enzymes:main | |
- reformat_pykofamsearch=pykofamsearch.reformat_pykofamsearch:main | |
- serialize_kofam_models=pykofamsearch.serialize_kofam_models:main | |
- subset_serialized_models=pykofamsearch.subset_serialized_models:main | |
run_exports: | |
- {{ pin_subpackage("pyhmmsearch", max_pin=None) }} | |
build: | |
script: {{ PYTHON }} -m pip install . -vvv --no-deps --no-build-isolation --no-cache-dir | |
number: 0 | |
noarch: python | |
entry_points: | |
- pykofamsearch=pykofamsearch.pykofamsearch:main | |
- reformat_enzymes=pykofamsearch.reformat_enzymes:main | |
- reformat_pykofamsearch=pykofamsearch.reformat_pykofamsearch:main | |
- serialize_kofam_models=pykofamsearch.serialize_kofam_models:main | |
- subset_serialized_models=pykofamsearch.subset_serialized_models:main | |
run_exports: | |
- {{ pin_subpackage("pykofamsearch", max_pin=None) }} |
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/pykofamsearch/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/pykofamsearch/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (5)
recipes/pykofamsearch/meta.yaml (5)
1-10
: LGTM: Package and source information are correctly defined.The package name, version, source URL, and SHA256 checksum are all properly specified. The use of Jinja2 templating for dynamic values is a good practice.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
42-48
: LGTM: About section is complete and well-defined.The about section provides all necessary metadata including the home page, summary, license information, and development URL. The inclusion of the license file is good practice.
1-2
: Note: Ignore yamllint syntax error for '%' character.The yamllint tool reports a syntax error for the '%' character at the start of the file. This is a false positive and can be safely ignored. The '%' character is valid in this context as it's part of the Jinja2 templating syntax, which is commonly used in conda-forge recipes.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
34-40
:⚠️ Potential issueFix typo in the last test command.
The approach of testing the help output for each entry point is a good basic test. However, there's a typo in the last test command:
- - subset_serialized_models --helps + - subset_serialized_models --helpPlease correct this to ensure all tests pass.
24-32
: LGTM: Requirements are well-defined, but consider verifying version constraints.The host and run requirements are correctly specified. The use of minimum version constraints for the run requirements is a good practice for ensuring compatibility while allowing for updates.
However, it's worth verifying if the specified versions are the most up-to-date compatible versions, especially for
pyhmmer
which has a very specific minimum version (0.10.12).To help verify the latest compatible versions, you can run the following script:
#!/bin/bash # Description: Check latest versions of dependencies pip install yq echo "Latest pyhmmer version:" curl -s https://pypi.org/pypi/pyhmmer/json | jq -r '.info.version' echo "Latest pandas version:" curl -s https://pypi.org/pypi/pandas/json | jq -r '.info.version' echo "Latest tqdm version:" curl -s https://pypi.org/pypi/tqdm/json | jq -r '.info.version'
recipes/pykofamsearch/meta.yaml
Outdated
build: | ||
script: {{ PYTHON }} -m pip install . -vvv --no-deps --no-build-isolation --no-cache-dir | ||
number: 0 | ||
noarch: python | ||
entry_points: | ||
- pykofamsearch=pykofamsearch.pykofamsearch:main | ||
- reformat_enzymes=pykofamsearch.reformat_enzymes:main | ||
- reformat_pykofamsearch=pykofamsearch.reformat_pykofamsearch:main | ||
- serialize_kofam_models=pykofamsearch.serialize_kofam_models:main | ||
run_exports: | ||
- {{ pin_subpackage("pykofamsearch", max_pin=None) }} |
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
Consider revising the pinning strategy for pykofamsearch
.
The current max_pin=None
allows any version of pykofamsearch
to satisfy the dependency, which might be too permissive. Given the date-based versioning, it's advisable to implement a more restrictive pinning strategy to ensure compatibility and stability.
- Review and define a specific pinning range that aligns with your project's compatibility requirements.
🔗 Analysis chain
LGTM: Build section is well-defined, but consider reviewing the pinning strategy.
The build script, build number, noarch
specification, and entry points are all correctly defined. The run_exports
section is included, which is good for maintaining consistency across dependent packages.
However, consider if max_pin=None
is the desired pinning strategy for this package. This strategy means that any version of pykofamsearch
will satisfy the dependency, which might be too permissive. You might want to use a more restrictive pinning strategy depending on your versioning scheme and backwards compatibility guarantees.
To help decide on the appropriate pinning strategy, you can review the package's versioning history and compatibility guarantees. Here's a script to check the available versions:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check available versions of pykofamsearch on PyPI
pip install yq
curl -s https://pypi.org/pypi/pykofamsearch/json | jq -r '.releases | keys[]' | sort -V
Length of output: 468
Script:
#!/bin/bash
# Description: Check available versions of pykofamsearch on PyPI using jq
curl -s https://pypi.org/pypi/pykofamsearch/json | jq -r '.releases | keys[]' | sort -V
Length of output: 149
@jolespin apparently, some (or all) of the "secondary" commands (such as |
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/pykofamsearch/meta.yaml (2)
1-6
: Consider documenting the versioning strategy.The package uses date-based versioning (YYYY.MM.DD), which could lead to frequent updates. Consider documenting the version compatibility guarantees in the package description to help users manage dependencies effectively.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
43-49
: Consider adding more package metadata.The about section could benefit from additional metadata fields such as:
doc_url
for documentationdescription
for a more detailed package descriptionauthors
to credit the package maintainers
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/pykofamsearch/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/pykofamsearch/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (4)
recipes/pykofamsearch/meta.yaml (4)
8-10
: LGTM: Source configuration is well-defined.The PyPI URL template and SHA256 checksum are correctly specified.
35-41
: Verify help command functionality for all entry points.Based on the PR comments, there were issues with the
--help
parameter for some secondary commands. Please verify that all these commands now support the--help
parameter correctly before merging.Would you like assistance in generating a test script to verify this functionality?
25-33
: Consider more specific version constraints.The current version constraints are quite broad:
pandas >=2
could include future major versions that might introduce breaking changestqdm >=4
similarly allows any version above 4Consider using more specific version ranges (e.g.,
pandas >=2,<3
) to prevent potential compatibility issues.
12-23
: Review pip installation flags.The current pip installation flags might be too restrictive:
--no-deps
skips installing dependencies, which could cause issues if the package has build-time dependencies--no-build-isolation
disables build isolation, which might lead to inconsistent buildsConsider if these flags are necessary for your use case.
@jolespin I think that was my mistake. Sorry for that. |
@BiocondaBot please add label |
This PR adds
pykofamsearch
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>
.