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 pyhmmsearch #51419

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Add pyhmmsearch #51419

wants to merge 9 commits into from

Conversation

apcamargo
Copy link
Contributor

This PR adds pyhmmsearch, by @jolespin.


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 16, 2024

📝 Walkthrough
📝 Walkthrough

Walkthrough

The pull request introduces a new meta.yaml file for the pyhmmsearch package, detailing its metadata, dependencies, build instructions, and testing commands. The package is named "pyhmmsearch" and is assigned the version "2024.7.29". The source section includes a URL for downloading the package from PyPI, along with a SHA256 checksum for verification. The build section specifies a script for installation using pip, indicating no dependencies or build isolation are required. Entry points for three scripts—pyhmmsearch.py, reformat_pyhmmsearch.py, and serialize_hmm_models.py—are commented out. The requirements section lists dependencies for both host and runtime environments, including Python, pip, pyhmmer, pandas, and tqdm. Testing commands are provided to verify the accessibility and functionality of the main scripts, although they are also commented out. The about section offers additional project information, including the home URL, a summary, license type (MIT), and a reference to the license file.

Possibly related PRs

  • Update meta.yaml for spec2vec #49857: The changes in the meta.yaml file for the spec2vec package involve updates to the build script and requirements, which are similar to the modifications made in the meta.yaml for pyhmmsearch.
  • Update panacus recipe to add cmake and c++ dependency #50982: The meta.yaml updates for the panacus package include changes to the build requirements and script, aligning with the changes made in the pyhmmsearch package's meta.yaml.
  • Update PhyloAcc recipe #51090: The meta.yaml for phyloacc includes updates to the requirements and build conditions, relevant to the changes made in the pyhmmsearch package.
  • add sipros #51091: The addition of the sipros package involves a new meta.yaml file that outlines dependencies and build instructions, similar to the pyhmmsearch package's structure.
  • Update pyhmmer to 0.10.15 #51257: The update to the pyhmmer package's meta.yaml includes version and checksum updates, which are also present in the pyhmmsearch package's meta.yaml.
  • Add patchify #51268: The patchify package's meta.yaml introduces a new package with a structured build process, akin to the pyhmmsearch package's configuration.
  • Add FMSI #51277: The fmsi package's updates in the meta.yaml reflect changes in dependencies and build requirements, paralleling the modifications in the pyhmmsearch package.
  • Add sopa package #51367: The sopa package's meta.yaml includes a new configuration that mirrors the structure and requirements outlined in the pyhmmsearch package's meta.yaml.
  • [biobb_analysis] update 5.0.0 #51431: The updates to the biobb_analysis package's meta.yaml involve version increments and dependency adjustments, similar to the changes made in the pyhmmsearch 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: 1

🧹 Outside diff range and nitpick comments (4)
recipes/pyhmmsearch/meta.yaml (4)

1-6: LGTM! Consider using a more conventional version format.

The package name and version are correctly defined using Jinja2 templating. However, the version format (YYYY.MM.DD) is unconventional. While it's valid, consider using a more standard semantic versioning format (e.g., MAJOR.MINOR.PATCH) for better compatibility with tools and easier version comparison.

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)


23-31: LGTM! Consider version pinning for pandas and tqdm.

The requirements section is well-defined with appropriate dependencies. The version pinning for pyhmmer is good for ensuring compatibility. However, consider adding version constraints for pandas and tqdm to prevent potential compatibility issues in the future.

Example:

  run:
    - python
    - pyhmmer >=0.10.12
    - pandas >=1.0.0
    - tqdm >=4.0.0

Replace the version numbers with the minimum versions that your package is known to work with.


33-37: LGTM! Consider adding more comprehensive tests.

The current test commands verify that the entry point scripts are accessible and can run without errors, which is a good start. However, to ensure the package's functionality, consider adding more comprehensive tests. For example:

  1. Test with a small input file to verify the core functionality.
  2. Check if the package can handle edge cases (e.g., empty input, large input).
  3. Verify the output format of each script.

Example additional test:

  commands:
    - pyhmmsearch.py -h
    - reformat_pyhmmsearch.py -h
    - serialize_hmm_models.py -h
    - pyhmmsearch.py --version
    - python -c "import pyhmmsearch; assert pyhmmsearch.__version__ == '{{ version }}'"

39-43: LGTM! Consider adding maintainer information.

The about section provides essential information about the package, including the home URL, summary, license type, and license file reference. This is excellent for users and potential contributors. To further improve it, consider adding maintainer information:

about:
  home: https://github.com/new-atlantis-labs/pyhmmsearch-stable
  summary: Fast implementation of HMMSEARCH optimized for high-memory systems using PyHmmer
  license: MIT
  license_file: LICENSE
  maintainers:
    - your_github_username

Replace your_github_username with the appropriate maintainer information.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between a4d30f9 and 410aef6.

📒 Files selected for processing (1)
  • recipes/pyhmmsearch/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/pyhmmsearch/meta.yaml

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

🔇 Additional comments (3)
recipes/pyhmmsearch/meta.yaml (3)

8-10: LGTM! Source URL and checksum are correctly defined.

The source URL is properly constructed using Jinja2 templating, pointing to the correct PyPI package. The inclusion of the SHA256 checksum is excellent for ensuring package integrity during installation.


1-1: Ignore yamllint error for Jinja2 syntax.

The yamllint tool reports a syntax error for the '%' character at the beginning of the file. This is a false positive as the '%' character is part of the Jinja2 templating syntax, which is correctly used in conda-forge recipes. No action is required.

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)


1-43: Overall, the meta.yaml file is well-structured and follows Bioconda guidelines.

The package definition is comprehensive and adheres to best practices. Minor suggestions for improvement include:

  1. Considering a more conventional version format.
  2. Verifying the necessity of build script flags.
  3. Adding version constraints for pandas and tqdm.
  4. Implementing more comprehensive tests.
  5. Adding maintainer information.

These suggestions are not critical and the current implementation is acceptable. Great job on creating this recipe for pyhmmsearch!

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

Comment on lines 12 to 20
build:
script: {{ PYTHON }} -m pip install . -vv --no-deps --no-build-isolation
number: 0
entry_points:
- pyhmmsearch.py=pyhmmsearch/pyhmmsearch.py
- reformat_pyhmmsearch.py=reformat_pyhmmsearch.py
- serialize_hmm_models.py=serialize_hmm_models.py
run_exports:
- {{ pin_subpackage("pyhmmsearch", max_pin="x.x") }}
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

Potential Issues with --no-deps --no-build-isolation Flags

The use of --no-deps --no-build-isolation in the build script may prevent necessary dependencies from being installed during the build process. Since setup.py specifies install_requires, omitting dependencies could lead to build failures or runtime issues if required packages are not already present in the environment.

🔗 Analysis chain

LGTM! Verify the necessity of build script flags.

The build section is well-defined, with correct entry points and run_exports implementation. However, please verify if the --no-deps --no-build-isolation flags in the build script are necessary for this package. These flags prevent pip from installing or building dependencies, which might lead to issues if the package requires any build-time dependencies.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if the package has any build-time dependencies that might be affected by the --no-deps flag.

# Test: Search for setup.py or pyproject.toml to check build-time dependencies
fd -e py -e toml '(setup|pyproject)' | xargs cat

Length of output: 4708

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: 3

🧹 Outside diff range and nitpick comments (1)
recipes/pyhmmsearch/meta.yaml (1)

40-44: LGTM! Consider adding maintainer information.

The about section is well-defined with all necessary information. As an enhancement, consider adding maintainer information to make it easier for users to reach out with questions or issues.

You could add:

  dev_url: https://github.com/new-atlantis-labs/pyhmmsearch-stable
  doc_url: https://github.com/new-atlantis-labs/pyhmmsearch-stable#readme
  maintainers:
    - your_github_username

Replace your_github_username with the appropriate maintainer's GitHub username.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 410aef6 and d50a150.

📒 Files selected for processing (1)
  • recipes/pyhmmsearch/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/pyhmmsearch/meta.yaml

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

🔇 Additional comments (3)
recipes/pyhmmsearch/meta.yaml (3)

1-6: Verify the version format.

The version "2024.7.29" uses a date-based format (YYYY.MM.DD). While this is not incorrect, it's uncommon and might not align with semantic versioning principles. Consider if this accurately represents the package's versioning scheme.

Can you confirm if this version format is intentional and aligns with the upstream project's versioning?

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)


8-10: LGTM! Source section is well-defined.

The source URL is correctly constructed using Jinja2 variables, and the inclusion of a SHA256 checksum ensures package integrity.


16-19: Verify the correctness of entry points.

The entry points seem to be defined correctly, but it's important to ensure they match the actual script locations in the package. Please verify that:

  1. pyhmmsearch.py is indeed located at pyhmmsearch/pyhmmsearch.py
  2. reformat_pyhmmsearch.py is in the root directory
  3. serialize_hmm_models.py is in the root directory

Can you confirm that these entry points correctly reflect the package structure?

sha256: 5134759a9053a9c3f52ab6c3d672094915292d431f734074d1d33021138c75ec

build:
script: {{ PYTHON }} -m pip install . -vv --no-deps --no-build-isolation
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

Reconsider the use of --no-deps --no-build-isolation flags.

The build script uses --no-deps --no-build-isolation flags, which may prevent necessary dependencies from being installed during the build process. This could lead to build failures or runtime issues if required packages are not present in the environment.

Consider removing these flags unless there's a specific reason for their inclusion. If they are necessary, please provide a justification in a comment.

-  script: {{ PYTHON }} -m pip install . -vv --no-deps --no-build-isolation
+  script: {{ PYTHON }} -m pip install . -vv
📝 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
script: {{ PYTHON }} -m pip install . -vv --no-deps --no-build-isolation
script: {{ PYTHON }} -m pip install . -vv

Comment on lines +24 to +32
requirements:
host:
- python
- pip
run:
- python
- pyhmmer >=0.10.12
- pandas
- tqdm
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding version constraints for pandas and tqdm.

While you've specified a version constraint for pyhmmer, the pandas and tqdm dependencies don't have any version specifications. To ensure compatibility and reproducibility, it's generally a good practice to specify version constraints for all dependencies.

Consider adding version constraints like this:

  run:
    - python
    - pyhmmer >=0.10.12
-   - pandas
-   - tqdm
+   - pandas >=1.0.0
+   - tqdm >=4.0.0

Please adjust the version numbers according to the minimum versions required by your 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
requirements:
host:
- python
- pip
run:
- python
- pyhmmer >=0.10.12
- pandas
- tqdm
requirements:
host:
- python
- pip
run:
- python
- pyhmmer >=0.10.12
- pandas >=1.0.0
- tqdm >=4.0.0

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 d50a150 and 5a69316.

📒 Files selected for processing (1)
  • recipes/pyhmmsearch/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/pyhmmsearch/meta.yaml

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

🔇 Additional comments (5)
recipes/pyhmmsearch/meta.yaml (5)

1-10: LGTM: Package and source information are well-defined.

The package name, version, source URL, and SHA256 checksum are correctly specified. This section adheres to the Bioconda recipe guidelines.

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)


40-44: LGTM: About section is complete and accurate.

The about section includes all necessary information: home URL, summary, license type, and license file reference. This adheres to the Bioconda recipe guidelines and provides clear information about the package.


1-44: Overall, the recipe is well-structured with some areas for improvement.

The meta.yaml file for pyhmmsearch follows the Bioconda recipe guidelines and includes all necessary sections. The package metadata, build instructions, dependencies, and basic tests are defined. However, there are a few areas that could be improved:

  1. Reconsider the use of --no-deps --no-build-isolation flags in the build script.
  2. Add version constraints for pandas and tqdm in the run requirements.
  3. Enhance the test section with more comprehensive tests.

Addressing these points will improve the robustness and maintainability of the recipe. Once these changes are made, the recipe should be ready for inclusion in the Bioconda repository.

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)


13-13: ⚠️ Potential issue

Reconsider the use of --no-deps --no-build-isolation flags.

The build script uses --no-deps --no-build-isolation flags, which may prevent necessary dependencies from being installed during the build process. This could lead to build failures or runtime issues if required packages are not present in the environment.

Consider removing these flags unless there's a specific reason for their inclusion. If they are necessary, please provide a justification in a comment.

-  script: {{ PYTHON }} -m pip install . -vv --no-deps --no-build-isolation
+  script: {{ PYTHON }} -m pip install . -vv

Likely invalid or redundant comment.


24-32: 🛠️ Refactor suggestion

Consider adding version constraints for pandas and tqdm.

While you've specified a version constraint for pyhmmer, the pandas and tqdm dependencies don't have any version specifications. To ensure compatibility and reproducibility, it's generally a good practice to specify version constraints for all dependencies.

Consider adding version constraints like this:

  run:
    - python
    - pyhmmer >=0.10.12
-   - pandas
-   - tqdm
+   - pandas >=1.0.0
+   - tqdm >=4.0.0

Please adjust the version numbers according to the minimum versions required by your package.

Likely invalid or redundant comment.

Comment on lines 34 to 38
test:
commands:
- pyhmmsearch.py -h
- reformat_pyhmmsearch.py -h
- serialize_hmm_models.py -h
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding more comprehensive tests.

The current test commands check if the scripts can be executed with the -h flag, which is a good basic check. However, more comprehensive tests could provide better assurance of package functionality.

Consider adding tests that:

  1. Execute the scripts with sample inputs
  2. Verify expected outputs
  3. Check for specific functionalities

For example, you could add:

  commands:
    - pyhmmsearch.py -h
    - reformat_pyhmmsearch.py -h
    - serialize_hmm_models.py -h
    - python -c "import pyhmmsearch; assert pyhmmsearch.__version__ == '{{ version }}'"
    # Add more specific functional tests here

This addition checks if the package can be imported and if the version is correct. Add more specific tests based on the package's functionality.

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 d1d836d and 88ac77a.

📒 Files selected for processing (2)
  • recipes/pyhmmsearch/meta.yaml (1 hunks)
  • recipes/sopa/meta.yaml (3 hunks)
🧰 Additional context used
🪛 yamllint
recipes/pyhmmsearch/meta.yaml

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

🔇 Additional comments (12)
recipes/pyhmmsearch/meta.yaml (6)

1-10: LGTM: Package and source sections are well-defined.

The package name, version, source URL, and sha256 checksum are correctly specified. This follows the best practices for Bioconda recipes.

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)


16-19: Clarify the status of commented-out entry points.

The entry points for pyhmmsearch.py, reformat_pyhmmsearch.py, and serialize_hmm_models.py are currently commented out. If these are intended to be executable scripts, they should be uncommented. Otherwise, if they are not needed, consider removing these lines entirely.

Please clarify the intended use of these scripts and update the configuration accordingly.


41-48: LGTM: About section is comprehensive and well-defined.

The about section provides all necessary information including the home URL, summary, license details, and additional URLs for development and documentation. This follows the best practices for Bioconda recipes and provides users with valuable information about the package.


1-1: Note: Ignore yamllint error for Jinja2 syntax.

The static analysis tool yamllint reports a syntax error for the Jinja2 template syntax ({% set ... %}). This is a false positive as Jinja2 template syntax is valid and commonly used in meta.yaml files for Conda recipes. No action is required to address this error.

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)


1-48: Summary: meta.yaml is well-structured with some areas for improvement.

Overall, the meta.yaml file for pyhmmsearch is well-structured and follows most Bioconda guidelines. Here's a summary of the review:

  1. Package and source sections are correctly defined.
  2. Build section needs attention regarding the use of --no-deps --no-build-isolation flags and clarification on entry points.
  3. Requirements section could benefit from adding version constraints for all dependencies.
  4. Test section could be enhanced with more comprehensive tests.
  5. About section is comprehensive and well-defined.

Please address the suggestions in the previous comments to improve the recipe. Once these changes are made, the package should be ready for inclusion in the Bioconda channel.

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)


23-31: 🛠️ Refactor suggestion

Consider adding version constraints for pandas and tqdm.

While you've specified a version constraint for pyhmmer, the pandas and tqdm dependencies don't have any version specifications. To ensure compatibility and reproducibility, it's generally a good practice to specify version constraints for all dependencies.

Consider adding version constraints like this:

  run:
    - python
    - pyhmmer >=0.10.12
-   - pandas
-   - tqdm
+   - pandas >=1.0.0
+   - tqdm >=4.0.0

Please adjust the version numbers according to the minimum versions required by your package.

Likely invalid or redundant comment.

recipes/sopa/meta.yaml (6)

16-16: Build script improvements approved.

The changes to the build script are beneficial:

  1. Adding --no-cache-dir ensures a clean build by preventing the use of cached packages.
  2. Increasing verbosity from -vv to -vvv provides more detailed output during installation, which can be helpful for debugging.

These modifications align with best practices for conda recipe builds.


61-61: Summary formatting improvement approved.

The addition of a period at the end of the summary is a minor but welcome improvement to the formatting. It enhances the readability of the package metadata.


Line range hint 1-69: Overall review: Changes approved with verification steps

All changes in this update to the sopa recipe have been reviewed and approved. The modifications include:

  1. Version update to 1.1.5
  2. Build script improvements for cleaner builds and better debugging
  3. Dependency optimization by switching to snakemake-minimal
  4. Minor formatting improvement in the summary
  5. Addition of dev_url for better package information

These changes collectively improve the package recipe and metadata. However, please ensure to complete the suggested verification steps:

  1. Confirm version consistency across the recipe and source URL
  2. Verify that snakemake-minimal provides all necessary functionality
  3. Check the accessibility of the provided dev_url

Once these verifications are complete, the recipe update should be ready for merging.


64-64: Addition of dev_url approved.

The inclusion of the dev_url field is a valuable addition to the package metadata. It provides users with a direct link to the project's documentation or development page, making it easier to find more information about the project.

Please verify that the URL "https://gustaveroussy.github.io/sopa" is correct and accessible. You can use the following command to check:

#!/bin/bash
# Verify dev_url accessibility
curl -sI https://gustaveroussy.github.io/sopa | head -n 1

This command will return the HTTP status code of the URL. Ensure it returns a 200 OK status.


Line range hint 2-2: Version update looks good.

The version has been updated to 1.1.5. This change is straightforward and appropriate.

Please ensure that this version matches the one in the source URL and the package's actual release. You can verify this with the following command:

✅ Verification successful

Version update verified.

The version is correctly set to 1.1.5 and matches the source URL.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify version consistency
grep -n "version = " recipes/sopa/meta.yaml
grep -n "sopa-" recipes/sopa/meta.yaml

Length of output: 207


40-40: Dependency change to snakemake-minimal approved.

The change from snakemake to snakemake-minimal is a good optimization that can reduce the package's footprint and dependencies. The version constraints remain the same, which maintains consistency.

Please verify that snakemake-minimal provides all the necessary functionality for the package. You can check this by running:

If the search returns any results, please review them to ensure that snakemake-minimal is sufficient for these use cases.

✅ Verification successful

Dependency change to snakemake-minimal verified.

No direct usage of snakemake was found in the sopa codebase. The existing references to snakemake-minimal ensure that all necessary functionalities are maintained.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if sopa uses any features specific to full snakemake
rg -i 'snakemake' --type python

Length of output: 149


Script:

#!/bin/bash
# Corrected script to check if 'sopa' uses any features specific to full snakemake
rg -i 'snakemake' --type python

Length of output: 62


Script:

#!/bin/bash
# Search for 'snakemake' usage in all Python files using a glob pattern
rg -i 'snakemake' --glob '*.py'

Length of output: 280

Comment on lines +33 to +39
test:
imports:
- pyhmmsearch
commands:
#- pyhmmsearch.py -h
#- reformat_pyhmmsearch.py -h
#- serialize_hmm_models.py -h
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance test coverage with more comprehensive tests.

The current test configuration only checks if the package can be imported. While this is a good start, more comprehensive tests could provide better assurance of package functionality. Consider adding tests that:

  1. Execute the scripts with sample inputs
  2. Verify expected outputs
  3. Check for specific functionalities

For example, you could add:

  commands:
    - pyhmmsearch.py -h
    - reformat_pyhmmsearch.py -h
    - serialize_hmm_models.py -h
    - python -c "import pyhmmsearch; assert pyhmmsearch.__version__ == '{{ version }}'"
    # Add more specific functional tests here

This addition checks if the scripts can be executed and if the package version is correct. Add more specific tests based on the package's functionality.

sha256: 5134759a9053a9c3f52ab6c3d672094915292d431f734074d1d33021138c75ec

build:
script: {{ PYTHON }} -m pip install . -vvv --no-deps --no-build-isolation --no-cache-dir
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

Reconsider the use of --no-deps --no-build-isolation flags.

The build script uses --no-deps --no-build-isolation flags, which may prevent necessary dependencies from being installed during the build process. This could lead to build failures or runtime issues if required packages are not present in the environment.

Consider removing these flags unless there's a specific reason for their inclusion. If they are necessary, please provide a justification in a comment.

-  script: {{ PYTHON }} -m pip install . -vvv --no-deps --no-build-isolation --no-cache-dir
+  script: {{ PYTHON }} -m pip install . -vvv
📝 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
script: {{ PYTHON }} -m pip install . -vvv --no-deps --no-build-isolation --no-cache-dir
script: {{ PYTHON }} -m pip install . -vvv

@apcamargo
Copy link
Contributor Author

@jolespin do you think you can use entrypoints to expose pyhmmsearch as an executable (see example here)? This is obviously a suggestion, but I wanted to put pyhmmsearch, pykofamsearch, kegg_pathway_profiler in BioConda and the way the executables are being exposed is not playing nicely.

@jolespin
Copy link

Yea definitely! I'll look into these tomorrow morning and try to implement them by the end of the week. Thanks for getting the ball rolling on the bioconda push.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants