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 recipe for sizemeup #51033

Merged
merged 4 commits into from
Sep 29, 2024
Merged

add recipe for sizemeup #51033

merged 4 commits into from
Sep 29, 2024

Conversation

rpetit3
Copy link
Member

@rpetit3 rpetit3 commented Sep 29, 2024

This PR adds a recipe for sizemeup a simple tool to get genome sizes based on species names


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>.

Summary by CodeRabbit

  • New Features

    • Introduced a setup script for easy installation and configuration of the sizemeup library.
    • Added a configuration file for sizemeup, detailing package information and dependencies.
  • Bug Fixes

    • Ensured proper permissions and naming for the main executable.
  • Documentation

    • Included metadata and integrity verification details in the new configuration file.

Copy link

coderabbitai bot commented Sep 29, 2024

📝 Walkthrough
📝 Walkthrough

Walkthrough

This pull request introduces the sizemeup library, which includes a build.sh script for installation and a meta.yaml file for package configuration. The build.sh script prepares necessary directories, installs the library using pip while avoiding dependencies, modifies permissions of the executable, and copies a genome size database file. The meta.yaml file specifies package metadata, dependencies, and testing requirements for version 1.0.2 of the library.

Changes

Files Change Summary
recipes/sizemeup/build.sh Introduced a script to automate the installation and setup of the sizemeup library.
recipes/sizemeup/meta.yaml Created a configuration file defining package metadata, dependencies, and testing requirements.

Possibly related PRs

  • Add recipe for consensify #50969: The meta.yaml file in this PR also defines package configuration, similar to the changes made in the meta.yaml for the sizemeup library, indicating a related context of package management.
  • recipe for pgrc #50973: The build.sh script introduced in this PR shares a similar purpose of automating the build process, akin to the build.sh script for sizemeup, suggesting a commonality in installation and setup methodologies for different libraries.

Suggested labels

please review & merge

Suggested reviewers

  • martin-g
  • apeltzer

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

@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 (6)
recipes/sizemeup/build.sh (3)

5-6: LGTM: Proper installation of sizemeup library.

The installation command is correct and uses appropriate flags for isolated installation. However, consider adding error handling to ensure the installation was successful.

You could add error handling like this:

-$PYTHON -m pip install --no-deps --ignore-installed -vv .
+$PYTHON -m pip install --no-deps --ignore-installed -vv . || exit 1

This will cause the script to exit if the installation fails, preventing further execution with an incomplete setup.


8-11: LGTM: Proper setup of executable and wrapper.

The script correctly sets up the main executable and wrapper. However, consider adding error checking for critical operations.

You could add error handling for critical operations:

-chmod 755 bin/sizemeup-bioconda
-mv $PREFIX/bin/sizemeup $PREFIX/bin/sizemeup-main
-cp -f bin/sizemeup-bioconda $PREFIX/bin/sizemeup
+chmod 755 bin/sizemeup-bioconda || exit 1
+mv $PREFIX/bin/sizemeup $PREFIX/bin/sizemeup-main || exit 1
+cp -f bin/sizemeup-bioconda $PREFIX/bin/sizemeup || exit 1

This ensures that the script exits if any of these critical operations fail.


13-14: LGTM: Proper copying of genome size database.

The script correctly copies the genome size database to the shared directory. However, consider adding a check to ensure the file exists before copying.

You could add a check for the database file:

-cp -f data/sizemeup-sizes.txt ${PREFIX}/share/sizemeup
+if [ -f data/sizemeup-sizes.txt ]; then
+    cp -f data/sizemeup-sizes.txt ${PREFIX}/share/sizemeup || exit 1
+else
+    echo "Error: Genome size database file not found" >&2
+    exit 1
+fi

This ensures that the script exits with an informative error message if the database file is missing.

recipes/sizemeup/meta.yaml (3)

1-10: LGTM! Consider using Jinja2 variable for GitHub repository.

The package and source sections are well-defined. The use of Jinja2 variables for the package name and version is a good practice.

Consider defining a Jinja2 variable for the GitHub repository to make future updates easier. For example:

{% set name = "sizemeup" %}
{% set version = "1.0.0" %}
{% set github_repo = "rpetit3/sizemeup" %}

package:
  name: {{ name|lower }}
  version: {{ version }}

source:
  url: https://github.com/{{ github_repo }}/archive/v{{ version }}.tar.gz
  sha256: f8307bd4c9fb7b404046c87e18f1811daeefe9e8d431c0ea55e6adc004008338
🧰 Tools
🪛 yamllint

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

(syntax)


12-16: Consider removing the run_exports section.

The build section is mostly well-defined. However, the run_exports section might not be necessary for this Python package.

For Python packages, run_exports is typically not needed unless the package provides compiled extensions or is meant to be linked against. Consider removing the run_exports section:

build:
  number: 0
  noarch: generic

If there's a specific reason for including run_exports, please clarify its necessity for this package.


46-48: LGTM! Add a newline at the end of the file.

The extra section correctly lists the recipe maintainer.

Add a newline at the end of the file to comply with YAML best practices and resolve the linter warning:

 extra:
   recipe-maintainers:
     - rpetit3
+
🧰 Tools
🪛 yamllint

[error] 48-48: no new line character at the end of file

(new-line-at-end-of-file)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 75dca3f and e2aebf7.

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

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

(syntax)


[error] 48-48: no new line character at the end of file

(new-line-at-end-of-file)

🔇 Additional comments (3)
recipes/sizemeup/build.sh (1)

1-3: LGTM: Correct shebang and directory setup.

The script starts with the correct shebang for a bash script and properly creates the necessary directories using the $PREFIX variable, which is consistent with Bioconda's build system.

recipes/sizemeup/meta.yaml (2)

40-44: LGTM! About section is well-defined.

The about section provides all the necessary information, including the home page URL, a concise summary, and correct license details.


1-2: Note: Ignore the YAML syntax error for Jinja2 templates.

The YAML linter reports a syntax error for the '%' characters at the beginning of the file. This is a false positive, as these are valid Jinja2 template markers used in Conda recipes.

No action is required to address this linter warning.

🧰 Tools
🪛 yamllint

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

(syntax)

Comment on lines +1 to +14
#!/bin/bash

mkdir -p $PREFIX/bin ${PREFIX}/share/sizemeup

# Install sizemup library
$PYTHON -m pip install --no-deps --ignore-installed -vv .

# move main executable, and replace with wrapper
chmod 755 bin/sizemeup-bioconda
mv $PREFIX/bin/sizemeup $PREFIX/bin/sizemeup-main
cp -f bin/sizemeup-bioconda $PREFIX/bin/sizemeup

# Copy genome size database (~100kb)
cp -f data/sizemeup-sizes.txt ${PREFIX}/share/sizemeup
Copy link

@coderabbitai coderabbitai bot Sep 29, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding overall error handling and logging.

While the script is functional and follows Bioconda conventions, it could benefit from some overall improvements:

  1. Add a logging mechanism to track the progress of the installation.
  2. Implement overall error handling to catch and report any unexpected issues.
  3. Consider adding a cleanup function to handle interruptions gracefully.

Here's an example of how you could implement these improvements:

#!/bin/bash

set -e  # Exit immediately if a command exits with a non-zero status.

log() {
    echo "[$(date +'%Y-%m-%d %H:%M:%S')] $1"
}

cleanup() {
    log "Cleaning up..."
    # Add any necessary cleanup steps here
}

trap cleanup EXIT

log "Starting sizemeup installation"

mkdir -p $PREFIX/bin ${PREFIX}/share/sizemeup
log "Created necessary directories"

log "Installing sizemeup library"
$PYTHON -m pip install --no-deps --ignore-installed -vv . || { log "Failed to install sizemeup library"; exit 1; }

log "Setting up executable and wrapper"
chmod 755 bin/sizemeup-bioconda || { log "Failed to set permissions for sizemeup-bioconda"; exit 1; }
mv $PREFIX/bin/sizemeup $PREFIX/bin/sizemeup-main || { log "Failed to move sizemeup executable"; exit 1; }
cp -f bin/sizemeup-bioconda $PREFIX/bin/sizemeup || { log "Failed to copy sizemeup-bioconda wrapper"; exit 1; }

log "Copying genome size database"
if [ -f data/sizemeup-sizes.txt ]; then
    cp -f data/sizemeup-sizes.txt ${PREFIX}/share/sizemeup || { log "Failed to copy genome size database"; exit 1; }
else
    log "Error: Genome size database file not found"
    exit 1
fi

log "sizemeup installation completed successfully"

This version of the script includes logging, error handling, and a cleanup function to make the installation process more robust and easier to debug.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm going to pass on this recommendation as it makes the build.sh way more complex then it really needs to be

Copy link

Choose a reason for hiding this comment

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

@rpetit3, understood, thank you for letting me know.

(_/)
( •_•)
💧


✏️ Learnings added
Learnt from: rpetit3
PR: bioconda/bioconda-recipes#51033
File: recipes/sizemeup/build.sh:1-14
Timestamp: 2024-09-29T21:59:41.620Z
Learning: In build scripts like `build.sh`, prefer simplicity and avoid adding unnecessary complexity such as detailed logging and error handling beyond what's necessary.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Comment on lines +28 to +38
test:
imports:
- sizemeup
commands:
- pip check
- sizemeup --version
- sizemeup --help
- sizemeup --species "Staphylococcus aureus"
- sizemeup-build --help
requires:
- pip
Copy link

@coderabbitai coderabbitai bot Sep 29, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

Remove undefined sizemeup-build command from test scripts

The sizemeup-build command is referenced in the test commands but does not exist in the codebase or documentation. Please remove this command from the test section to avoid confusion and potential test failures.

  • File: recipes/sizemeup/meta.yaml
  • Lines: 28-38
🔗 Analysis chain

LGTM! Please clarify the sizemeup-build command.

The test section is well-defined and comprehensive, covering import checks, pip check, and various CLI functionalities.

Could you please provide more information about the sizemeup-build command? It's not mentioned in the package summary, and it would be helpful to understand its purpose and ensure it's documented in the package's README or documentation.

To verify the existence and purpose of this command, we can check the project's documentation:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for sizemeup-build documentation
rg -i 'sizemeup-build' README.md || echo "No mention of sizemeup-build in README.md"

Length of output: 125


Script:

#!/bin/bash
# Description: Search for the implementation of 'sizemeup-build' in the codebase
rg -i 'sizemeup-build' src/ || echo "sizemeup-build command not found in the src directory"

Length of output: 189

Copy link
Member Author

Choose a reason for hiding this comment

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

AI growing pains, sizemeup-build does actually exist

Copy link

Choose a reason for hiding this comment

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

@rpetit3 Thank you for the clarification! I apologize for the oversight. I'll make sure to remember that sizemeup-build does indeed exist in the package.

(>_<) Sorry for the confusion.


✏️ Learnings added
Learnt from: rpetit3
PR: bioconda/bioconda-recipes#51033
File: recipes/sizemeup/meta.yaml:28-38
Timestamp: 2024-09-29T21:58:52.201Z
Learning: The `sizemeup-build` command exists in the `sizemeup` package and should be included in the test scripts where applicable.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Copy link

@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 (1)
recipes/sizemeup/meta.yaml (1)

1-48: Overall, the recipe is well-structured but requires a few adjustments.

The sizemeup recipe is generally well-constructed and aligns with the PR objectives. However, there are a few areas that need attention:

  1. Adjust the Python version constraint and host requirements as suggested in the requirements section.
  2. Remove the sizemeup-build command from the test section.

Once these changes are made, the recipe should be ready for inclusion in Bioconda. Please make the necessary adjustments and update the PR.

After making the changes:

  1. Ensure all tests pass.
  2. Issue the command @BiocondaBot please add label when the PR is ready for merging.

Thank you for contributing to Bioconda!

🧰 Tools
🪛 yamllint

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

(syntax)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between e2aebf7 and a40153a.

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

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

(syntax)

🔇 Additional comments (6)
recipes/sizemeup/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. The use of Jinja2 variables for the package name and version is a good practice for maintainability.

🧰 Tools
🪛 yamllint

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

(syntax)


12-17: LGTM! Build section is well-configured.

The build section is correctly set up with:

  • Appropriate build number (0 for a new package).
  • noarch: generic, which is suitable for Python packages without compiled extensions.
  • Proper use of run_exports for dependency management.

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

The about section provides all necessary metadata, including:

  • Home page URL
  • Concise summary of the tool's purpose
  • License type (MIT)
  • License file location

This information aligns well with the PR objectives and Bioconda guidelines.


46-48: LGTM! Extra section correctly specifies the recipe maintainer.

The extra section properly lists rpetit3 as the recipe maintainer, which aligns with the PR submitter information.


18-26: ⚠️ Potential issue

Reconsider Python version constraint and host requirements.

The requirements section needs adjustments to ensure broader compatibility and align with Bioconda best practices:

  1. The Python version constraint of >=3.12 is very recent and might limit the package's usability. Consider using a lower minimum version, such as >=3.7 or >=3.8, unless there's a specific need for Python 3.12 features.

  2. The inclusion of poetry in the host requirements is unusual for a Bioconda recipe. Typically, poetry is used for development and packaging, not for installation in the conda environment. Consider removing it unless it's absolutely necessary for the build process.

Suggested changes:

requirements:
  host:
    - python >=3.7
    - pip
  run:
    - python >=3.7
    - requests
    - rich-click >=1.6.0

If there are specific reasons for the current choices, please provide more context.


28-38: ⚠️ Potential issue

Remove undefined sizemeup-build command from test scripts

The sizemeup-build command is referenced in the test commands but does not exist in the codebase or documentation. Please remove this command from the test section to avoid confusion and potential test failures.

Suggested change:

test:
  imports:
    - sizemeup
  commands:
    - pip check
    - sizemeup --version
    - sizemeup --help
    - sizemeup --species "Staphylococcus aureus"
  requires:
    - pip

@rpetit3
Copy link
Member Author

rpetit3 commented Sep 29, 2024

@BiocondaBot please add label

@BiocondaBot BiocondaBot added the please review & merge set to ask for merge label Sep 29, 2024
@rpetit3
Copy link
Member Author

rpetit3 commented Sep 29, 2024

@BiocondaBot please fetch artifacts

@BiocondaBot
Copy link
Collaborator

Package(s) built are ready for inspection:

Arch Package Zip File / Repodata CI Instructions
noarch sizemeup-1.0.1-py312hdfd78af_0.tar.bz2 LinuxArtifacts.zip Azure
showYou may also use conda to install after downloading and extracting the zip file. From the LinuxArtifacts directory: conda install -c ./packages <package name>

Docker image(s) built:

Package Tag CI Install with docker
sizemeup 1.0.1--py312hdfd78af_0 Azure
showImages for Azure are in the LinuxArtifacts zip file above.gzip -dc LinuxArtifacts/images/sizemeup:1.0.1--py312hdfd78af_0.tar.gz | docker load

@rpetit3
Copy link
Member Author

rpetit3 commented Sep 29, 2024

Testing

Conda

The built in test confirmed conda is working

Docker

docker run --rm -u $(id -u):$(id -g) -v ${PWD}:/data quay.io/biocontainers/sizemeup:1.0.1--py312hdfd78af_0 sizemeup --version
sizemeup-main, version 1.0.1

docker run --rm -u $(id -u):$(id -g) -v ${PWD}:/data quay.io/biocontainers/sizemeup:1.0.1--py312hdfd78af_0 sizemeup --help

 Usage: sizemeup-main [OPTIONS]

 sizemeup - A simple tool to determine the genome size of an organism

╭─ Required Options ───────────────────────────────────────────────────────────╮
│ *  --species  -s  TEXT  The species to determine the size of [required]      │
│ *  --sizes    -z  TEXT  The built in sizes file to use                       │
│                         [default:                                            │
│                         /usr/local/bin/../share/sizemeup/sizemeup-sizes.txt] │
│                         [required]                                           │
╰──────────────────────────────────────────────────────────────────────────────╯
╭─ Additional Options ─────────────────────────────────────────────────────────╮
│ --outdir   -o  PATH  Directory to write output [default: ./]                 │
│ --prefix   -p  TEXT  Prefix to use for output files [default: sizemeup]      │
│ --silent             Only critical errors will be printed                    │
│ --verbose            Increase the verbosity of output                        │
│ --version  -V        Show the version and exit.                              │
│ --help               Show this message and exit.                             │
╰──────────────────────────────────────────────────────────────────────────────╯
docker run --rm -u $(id -u):$(id -g) -v ${PWD}:/data quay.io/biocontainers/sizemeup:1.0.1--py312hdfd78af_0 sizemeup --species "staphylococcus aureus" --outdir /data
PermissionError: [Errno 13] Permission denied: 'sizemeup-sizemeup.txt'

Need to fix issue on sizemeup end

Copy link

@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 a40153a and 7de952b.

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

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

(syntax)

🔇 Additional comments (6)
recipes/sizemeup/meta.yaml (6)

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

The package name, version, source URL, and SHA256 checksum are correctly specified. This ensures proper identification and integrity of the package source.

🧰 Tools
🪛 yamllint

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

(syntax)


12-16: LGTM: Build configuration is appropriate.

The build section is well-configured:

  • Build number is correctly set to 0 for the initial release.
  • The package is correctly marked as noarch: generic, which is suitable for Python packages without compiled extensions.
  • The use of run_exports is a good practice for managing dependencies across different builds.

28-38: LGTM: Comprehensive test section.

The test section is well-defined and thorough:

  • It includes import checks to ensure the package can be imported correctly.
  • The pip check command verifies that all dependencies are satisfied.
  • Various CLI functionalities are tested, including version display, help command, and a specific use case.
  • The sizemeup-build command is included, which has been confirmed to exist by the package author.

This comprehensive set of tests helps ensure the package's functionality and integrity.


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

The about section provides all necessary metadata:

  • The home page URL is correctly specified.
  • The summary accurately describes the tool's purpose.
  • The license (MIT) is clearly stated.
  • The license file location is provided.

This information aligns well with the PR objectives and provides users with essential details about the package.


46-48: LGTM: Maintainer information is correctly specified.

The extra section correctly identifies rpetit3 as the recipe maintainer, which aligns with the PR author information.


1-1: Note on static analysis warning.

The yamllint tool reported a syntax error for the '%' character at the beginning of the file. This is a false positive as the '%' is part of the Jinja2 template syntax commonly used in conda recipes. No action is required to address this warning.

🧰 Tools
🪛 yamllint

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

(syntax)

Comment on lines 18 to 26
requirements:
host:
- python >=3.12
- poetry
- pip
run:
- python >=3.12
- requests
- rich-click >=1.6.0
Copy link

@coderabbitai coderabbitai bot Sep 29, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Consider adjusting Python version constraint and host requirements.

While the requirements are generally well-defined, there are two points to consider:

  1. The Python version constraint of >=3.12 is very recent and might limit the package's usability. Unless there's a specific need for Python 3.12 features, consider using a lower minimum version, such as >=3.7 or >=3.8, to ensure broader compatibility.

  2. The inclusion of poetry in the host requirements is unusual for a Bioconda recipe. Typically, poetry is used for development and packaging, not for installation in the conda environment. Consider removing it unless it's absolutely necessary for the build process.

Suggested changes:

requirements:
  host:
    - python >=3.7
    - pip
  run:
    - python >=3.7
    - requests
    - rich-click >=1.6.0

If there are specific reasons for the current choices, please provide more context.

Copy link
Member Author

Choose a reason for hiding this comment

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

This one should include poetry

Copy link

Choose a reason for hiding this comment

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

@rpetit3, understood regarding including poetry in the host requirements. If sizemeup requires poetry for its build process, then including it is appropriate.

Regarding the Python version constraint of >=3.12, is that intentional? Since Python 3.12 is very recent, setting a lower minimum version like >=3.7 or >=3.8 might help reach a broader user base, unless sizemeup specifically requires features introduced in Python 3.12.


✏️ Learnings added
Learnt from: rpetit3
PR: bioconda/bioconda-recipes#51033
File: recipes/sizemeup/meta.yaml:0-0
Timestamp: 2024-09-29T22:52:49.536Z
Learning: The `sizemeup` package requires `poetry` in the host requirements for its build process.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: rpetit3
PR: bioconda/bioconda-recipes#51033
File: recipes/sizemeup/meta.yaml:28-38
Timestamp: 2024-09-29T21:58:52.399Z
Learning: The `sizemeup-build` command exists in the `sizemeup` package and should be included in the test scripts where applicable.

Copy link

@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 (1)
recipes/sizemeup/meta.yaml (1)

18-26: Consider removing poetry from host requirements.

The requirements are generally well-defined, but there's one point to consider:

The inclusion of poetry in the host requirements is unusual for a Bioconda recipe. Typically, poetry is used for development and packaging, not for installation in the conda environment. Consider removing it unless it's absolutely necessary for the build process.

Suggested change:

requirements:
  host:
    - python >=3.7
    - pip
  run:
    - python >=3.7
    - requests
    - rich-click >=1.6.0

If there's a specific reason for including poetry, please provide more context.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 7de952b and 024c8c7.

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

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

(syntax)

🔇 Additional comments (5)
recipes/sizemeup/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. The use of Jinja2 variables for the package name and version is a good practice for maintainability.

🧰 Tools
🪛 yamllint

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

(syntax)


12-16: LGTM! Build settings are appropriate.

The build configuration is well-defined:

  • The build number is correctly set to 0 for a new package.
  • The noarch: generic setting is appropriate for a Python package without compiled extensions.
  • The use of run_exports with pin_subpackage is a good practice for managing dependencies across minor version updates.

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

The about section provides all necessary metadata:

  • The home page correctly points to the GitHub repository.
  • The summary accurately describes the tool's purpose.
  • The license (MIT) is specified along with the license file location.

This information aligns well with the PR objectives and the package's GitHub repository.


46-48: LGTM! Maintainer information is correct.

The extra section correctly specifies rpetit3 as the recipe maintainer, which matches the PR submitter.


1-48: Overall, the recipe is well-constructed with a few minor issues to address.

The meta.yaml file for the sizemeup package is generally well-defined and follows Bioconda guidelines. Here's a summary of the review:

  1. Package, source, and build sections are correctly specified.
  2. Requirements are mostly appropriate, but consider removing poetry from host requirements unless it's necessary for the build process.
  3. The test section is comprehensive, but the specific use case test might fail due to a permission error. Consider adjusting this test as suggested in the previous comment.
  4. About and extra sections provide accurate metadata and maintainer information.

Once these minor issues are addressed, the recipe should be ready for inclusion in Bioconda. Great work on creating this recipe for sizemeup!

🧰 Tools
🪛 yamllint

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

(syntax)

Comment on lines +28 to +38
test:
imports:
- sizemeup
commands:
- pip check
- sizemeup --version
- sizemeup --help
- sizemeup --species "Staphylococcus aureus"
- sizemeup-build --help
requires:
- pip
Copy link

@coderabbitai coderabbitai bot Sep 29, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Consider adjusting the specific use case test.

The test section is comprehensive and well-structured. However, there's a potential issue to address:

The specific use case test on line 35:

- sizemeup --species "Staphylococcus aureus"

might fail due to the permission error mentioned in the PR comments. This error occurs when the tool attempts to write the output file.

Consider modifying this test to avoid the permission issue. You could:

  1. Add a temporary directory with write permissions for the test.
  2. Use a --dry-run option if available, to skip file writing.
  3. Redirect the output to /dev/null or a temporary file.

For example:

- sizemeup --species "Staphylococcus aureus" --output /tmp/sizemeup_test_output.txt

or

- sizemeup --species "Staphylococcus aureus" > /dev/null

Please choose the most appropriate option based on the tool's capabilities and the desired test coverage.

Copy link
Member Author

Choose a reason for hiding this comment

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

This does not affect the tests and was corrected in the source of sizemeup

Copy link

Choose a reason for hiding this comment

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

@rpetit3, thank you for the clarification. It's great to hear that the permission issue has been corrected in the source of sizemeup and does not affect the tests.

(・_・)?


✏️ Learnings added
Learnt from: rpetit3
PR: bioconda/bioconda-recipes#51033
File: recipes/sizemeup/meta.yaml:28-38
Timestamp: 2024-09-29T22:53:18.469Z
Learning: The permission issue in 'sizemeup --species "Staphylococcus aureus"' has been corrected in the source of sizemeup and does not affect the tests.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: rpetit3
PR: bioconda/bioconda-recipes#51033
File: recipes/sizemeup/meta.yaml:28-38
Timestamp: 2024-09-29T21:58:52.399Z
Learning: The `sizemeup-build` command exists in the `sizemeup` package and should be included in the test scripts where applicable.

If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!

@rpetit3
Copy link
Member Author

rpetit3 commented Sep 29, 2024

@BiocondaBot please fetch artifacts

@BiocondaBot
Copy link
Collaborator

Package(s) built are ready for inspection:

Arch Package Zip File / Repodata CI Instructions
noarch sizemeup-1.0.2-py312hdfd78af_0.tar.bz2 LinuxArtifacts.zip Azure
showYou may also use conda to install after downloading and extracting the zip file. From the LinuxArtifacts directory: conda install -c ./packages <package name>

Docker image(s) built:

Package Tag CI Install with docker
sizemeup 1.0.2--py312hdfd78af_0 Azure
showImages for Azure are in the LinuxArtifacts zip file above.gzip -dc LinuxArtifacts/images/sizemeup:1.0.2--py312hdfd78af_0.tar.gz | docker load

@rpetit3
Copy link
Member Author

rpetit3 commented Sep 29, 2024

Testing again

docker run --rm -u $(id -u):$(id -g) -v ${PWD}:/data quay.io/biocontainers/sizemeup:1.0.2--py312hdfd78af_0 sizemeup --species "staphylococcus aureus" --outdir /data
2024-09-29 22:49:23 INFO     2024-09-29 22:49:23:root:INFO - Reading utils.py:31
                             genome sizes from
                             /usr/local/bin/../share/sizemeup/sizeme
                             up-sizes.txt
                    INFO     2024-09-29 22:49:23:root:INFO -     sizemeup.py:105
                             Found the size of staphylococcus
                             aureus in the sizes file
                    INFO     2024-09-29 22:49:23:root:INFO -     sizemeup.py:106
                             {'name': 'Staphylococcus aureus',
                             'tax_id': '1280', 'size':
                             '2800000', 'source': 'ncbi',
                             'method': 'manually-set'}
                           Query Result
┏━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━┳━━━━━━━━━┳━━━━━━━━┳━━━━━━━━━━━━━━┓
┃ Name                  ┃ TaxID ┃ Size    ┃ Source ┃ Method       ┃
┡━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━╇━━━━━━━━━╇━━━━━━━━╇━━━━━━━━━━━━━━┩
│ staphylococcus aureus │ 1280  │ 2800000 │ ncbi   │ manually-set │
└───────────────────────┴───────┴─────────┴────────┴──────────────┘
Writing the genome size to /data/sizemeup-sizemeup.txt

Good to go!

@mbhall88 mbhall88 merged commit f0b880c into bioconda:master Sep 29, 2024
7 checks passed
@rpetit3 rpetit3 deleted the rp3-add-sizemeup branch September 29, 2024 23:32
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.

3 participants