Skip to content
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

Merged
merged 4 commits into from
Oct 22, 2024
Merged

Add pykofamsearch #51537

merged 4 commits into from
Oct 22, 2024

Conversation

apcamargo
Copy link
Contributor

This PR adds pykofamsearch


Please read the guidelines for Bioconda recipes before opening a pull request (PR).

General instructions

  • If this PR adds or updates a recipe, use "Add" or "Update" appropriately as the first word in its title.
  • New recipes not directly relevant to the biological sciences need to be submitted to the conda-forge channel instead of Bioconda.
  • PRs require reviews prior to being merged. Once your PR is passing tests and ready to be merged, please issue the @BiocondaBot please add label command.
  • Please post questions on Gitter or ping @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:

build:
  run_exports:
    - ...

with ... being one of:

Case run_exports statement
semantic versioning {{ pin_subpackage("myrecipe", max_pin="x") }}
semantic versioning (0.x.x) {{ pin_subpackage("myrecipe", max_pin="x.x") }}
known breakage in minor versions {{ pin_subpackage("myrecipe", max_pin="x.x") }} (in such a case, please add a note that shortly mentions your evidence for that)
known breakage in patch versions {{ pin_subpackage("myrecipe", max_pin="x.x.x") }} (in such a case, please add a note that shortly mentions your evidence for that)
calendar versioning {{ pin_subpackage("myrecipe", max_pin=None) }}

while replacing "myrecipe" with either name if a name|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 Merge the master branch into a PR.
@BiocondaBot please add label Add the please review & merge label.
@BiocondaBot please fetch artifacts Post links to CI-built packages/containers.
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>.

Copy link
Contributor

coderabbitai bot commented Oct 21, 2024

📝 Walkthrough
📝 Walkthrough

Walkthrough

The pull request introduces a new file, meta.yaml, for the pykofamsearch package, which outlines its configuration details. This file specifies the package's name as "pykofamsearch" and its version as "2024.10.20". It includes the source URL and a SHA256 checksum for verification. The build section indicates that the package is architecture-independent and details the installation command using pip. Multiple entry points are defined, allowing users to execute various functionalities directly from the command line. The requirements section lists necessary dependencies, including Python, pip, and libraries such as pyhmmer, pandas, and tqdm, along with their version constraints. Additionally, the test section provides commands to verify the package's functionality by checking the accessibility of the entry points. The about section contains metadata about the package, such as its homepage, a brief summary, licensing information, and a development URL for further resources.

Possibly related PRs

  • Add KmerCamel #51245: The meta.yaml file for kmercamel also uses Jinja2 templating for dynamic configuration, similar to the changes made in the meta.yaml for pykofamsearch.
  • Add seqspec #51326: The meta.yaml for seqspec includes similar elements such as dynamic versioning and entry points, aligning with the structure and purpose of the meta.yaml in the main PR for pykofamsearch.
  • Add pyhmmsearch #51490: The update to pyhmmsearch's meta.yaml includes changes to the version and source URL, reflecting a similar maintenance and configuration style as seen in the pykofamsearch package's meta.yaml.

Suggested labels

please review & merge

Suggested reviewers

  • mencian

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Files that changed from the base of the PR and between 6b3870a and 80eb1ad.

📒 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 warning

The 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 or pyproject.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.

Comment on lines 35 to 41
test:
commands:
- pykofamsearch --help
- reformat_enzymes --help
- reformat_pykofamsearch --help
- serialize_kofam_models --help
- subset_serialized_models --helps
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
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

Comment on lines 12 to 23
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) }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
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) }}

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Files that changed from the base of the PR and between 28a5e92 and 5c19018.

📒 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 issue

Fix 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 --help

Please 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'

Comment on lines 12 to 22
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) }}
Copy link
Contributor

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

@apcamargo
Copy link
Contributor Author

@jolespin apparently, some (or all) of the "secondary" commands (such as serialize_kofam_models) don't have a --help parameter, so the tests are failing. Do you expose the help dialogue some other way?

@jolespin
Copy link

I tried a fresh install but it was working (screenshot) do you know what else it could be?

image

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 documentation
  • description for a more detailed package description
  • authors to credit the package maintainers
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 5c19018 and 9198b14.

📒 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 changes
  • tqdm >=4 similarly allows any version above 4

Consider 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 builds

Consider if these flags are necessary for your use case.

@apcamargo
Copy link
Contributor Author

@jolespin I think that was my mistake. Sorry for that.

@apcamargo
Copy link
Contributor Author

@BiocondaBot please add label

@BiocondaBot BiocondaBot added the please review & merge set to ask for merge label Oct 22, 2024
@mencian mencian merged commit ec52a12 into bioconda:master Oct 22, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
please review & merge set to ask for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants