-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add in new package mutamr #51139
Add in new package mutamr #51139
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request involve the addition of the GNU General Public License (GPL) Version 3 to the Changes
Possibly related PRs
Suggested labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
recipes/mutamr/meta.yaml (2)
13-20
: LGTM: Build configuration is well-defined with a minor suggestion.The build section is correctly configured:
- 'noarch: python' is appropriate for a pure Python package.
- The entry point is properly defined.
- The run_exports section ensures correct version pinning.
- The pip installation command is correct.
Consider adding a comment explaining the purpose of the
--no-deps
flag in the pip installation command, as it's not immediately obvious why dependencies are not being installed.
36-40
: LGTM: Basic tests are in place, with a suggestion for improvement.The test section includes appropriate basic tests:
- An import test to ensure the package can be imported.
- A command test to verify the CLI functionality.
Consider adding a more comprehensive test that exercises the main functionality of the package. For example:
commands: - mutamr --help - mutamr --version - mutamr run --test-data # Assuming there's a test data optionThis would provide more confidence in the package's core functionality.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- recipes/mutamr/LICENSE (1 hunks)
- recipes/mutamr/meta.yaml (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- recipes/mutamr/LICENSE
🧰 Additional context used
🪛 yamllint
recipes/mutamr/meta.yaml
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (6)
recipes/mutamr/meta.yaml (6)
1-7
: LGTM: Package metadata is well-defined.The package name, version, and SHA256 checksum are correctly set using Jinja2 variables. This approach enhances maintainability and ensures package integrity.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
9-11
: LGTM: Source URL and checksum are correctly defined.The source URL is properly constructed using Jinja2 variables, following the PyPI standard format. The SHA256 checksum is consistently referenced from its earlier definition.
42-47
: LGTM: About section is comprehensive and well-defined.The about section provides all necessary metadata:
- The home URL correctly points to the GitHub repository.
- License information is complete, including the license type, file, and family.
- The summary concisely describes the package's purpose.
49-51
: LGTM: Recipe maintainer is correctly specified.The extra section properly lists the recipe maintainer, which is crucial for package maintenance and support.
1-3
: Note: Ignore yamllint syntax error for Jinja2 templates.The yamllint tool reports a syntax error for the '%' character at the beginning of the file. This is a false positive as these lines use Jinja2 template syntax, which is standard in Conda recipe files. The syntax is correct and should be ignored.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
1-51
: Overall: Well-structured Conda recipe for the new 'mutamr' package.This recipe is well-crafted and aligns with the PR objectives of introducing a new package for creating a simple VCF for AMR calling. It follows Conda packaging best practices, including:
- Proper use of Jinja2 variables for maintainability.
- Correct specification of build requirements and run dependencies.
- Inclusion of basic tests for package functionality.
- Comprehensive metadata in the 'about' section.
The minor suggestions provided in the review comments will further improve the recipe's quality and maintainability.
🧰 Tools
🪛 yamllint
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
recipes/mutamr/meta.yaml
Outdated
requirements: | ||
host: | ||
- python >=3.10 | ||
- pip | ||
run: | ||
- python >=3.10 | ||
- bwa =0.7.18 | ||
- delly =1.2.8 | ||
- freebayes =1.3.8 | ||
- bcftool =1.20 | ||
- samtools =1.20 | ||
- samclip =0.4.0 | ||
- snpeff =5.2 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM: Requirements are well-defined, with a minor formatting issue.
The host and run requirements are appropriately specified:
- Host requirements (Python >=3.10 and pip) are correct for building a Python package.
- Run requirements include necessary bioinformatics tools with pinned versions, ensuring reproducibility.
There's a trailing space on line 35. Please remove it to adhere to YAML best practices:
-
+
📝 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.
requirements: | |
host: | |
- python >=3.10 | |
- pip | |
run: | |
- python >=3.10 | |
- bwa =0.7.18 | |
- delly =1.2.8 | |
- freebayes =1.3.8 | |
- bcftool =1.20 | |
- samtools =1.20 | |
- samclip =0.4.0 | |
- snpeff =5.2 | |
requirements: | |
host: | |
- python >=3.10 | |
- pip | |
run: | |
- python >=3.10 | |
- bwa =0.7.18 | |
- delly =1.2.8 | |
- freebayes =1.3.8 | |
- bcftool =1.20 | |
- samtools =1.20 | |
- samclip =0.4.0 | |
- snpeff =5.2 | |
🧰 Tools
🪛 yamllint
[error] 35-35: trailing spaces
(trailing-spaces)
- bwa =0.7.18 | ||
- delly =1.2.8 | ||
- freebayes =1.3.8 | ||
- bcftools =1.20 | ||
- samtools =1.20 | ||
- samclip =0.4.0 | ||
- snpeff =5.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are such strict pins absolutely necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe so - in order to ensure expected behaviour these dependencies have been tested and are 1. compatible (cause no clashes or other issues upon installation) and 2. ensure that the resulting files generated are as expected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
recipes/mutamr/meta.yaml (1)
13-20
: LGTM: Build configuration is well-defined, with a suggestion for improvement.The build section is correctly configured:
noarch: python
is appropriate for a Python package.- The entry point is properly defined.
- The
run_exports
section ensures proper version pinning.- The installation script uses
pip
with appropriate flags for a conda environment.Consider adding
--ignore-installed
to the pip install command to ensure a clean installation:- script: "{{ PYTHON }} -m pip install . --no-deps --no-build-isolation --no-cache-dir -vvv" + script: "{{ PYTHON }} -m pip install . --no-deps --ignore-installed --no-build-isolation --no-cache-dir -vvv"This can help prevent potential conflicts with existing installations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/mutamr/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/mutamr/meta.yaml
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (7)
recipes/mutamr/meta.yaml (7)
1-7
: LGTM: Package metadata is well-defined.The package metadata section is correctly structured using Jinja2 templating for improved maintainability. The package name, version, and SHA256 checksum are properly defined.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
9-11
: LGTM: Source URL is correctly defined.The source URL is properly constructed using Jinja2 templating, ensuring consistency with the package name and version. This follows the standard PyPI format for package downloads.
36-40
: LGTM: Test section is properly configured.The test section includes appropriate checks:
- An import test for the
mutamr
module.- A command test to verify the
mutamr
command-line interface.These tests will help ensure the package is correctly installed and functional.
42-48
: LGTM: About section provides comprehensive package information.The about section is well-structured and includes all necessary information:
- Home page URL is correctly specified.
- License information is complete, including the license file and family.
- A concise summary describes the package's functionality.
- The dev_url is included, providing a link to the development repository.
This metadata will be helpful for users and package managers.
50-52
: LGTM: Recipe maintainer is correctly specified.The extra section properly lists the recipe maintainer (kristyhoran). This information is important for package management and future updates.
1-3
: Note on static analysis warning: False positive for Jinja2 syntax.The static analysis tool reported a syntax error for the '%' character at the beginning of the file. This is a false positive, as the '%' character is valid and necessary for Jinja2 templating in conda recipe files. No action is required to address this warning.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
22-35
:⚠️ Potential issueRequirements are well-defined, with a minor formatting issue.
The host and run requirements are appropriately specified:
- Host requirements (Python >=3.10 and pip) are correct for building a Python package.
- Run requirements include necessary bioinformatics tools with pinned versions, ensuring reproducibility.
There's a trailing space on line 35. Please remove it to adhere to YAML best practices:
- +🧰 Tools
🪛 yamllint
[error] 35-35: trailing spaces
(trailing-spaces)
Add in a new package for creating a simple vcf for AMR calling
Please read the guidelines for Bioconda recipes before opening a pull request (PR).
General instructions
@BiocondaBot please add label
command.@bioconda/core
in a comment.Instructions for avoiding API, ABI, and CLI breakage issues
Conda is able to record and lock (a.k.a. pin) dependency versions used at build time of other recipes.
This way, one can avoid that expectations of a downstream recipe with regards to API, ABI, or CLI are violated by later changes in the recipe.
If not already present in the meta.yaml, make sure to specify
run_exports
(see here for the rationale and comprehensive explanation).Add a
run_exports
section like this:with
...
being one of:{{ pin_subpackage("myrecipe", max_pin="x") }}
{{ pin_subpackage("myrecipe", max_pin="x.x") }}
{{ pin_subpackage("myrecipe", max_pin="x.x") }}
(in such a case, please add a note that shortly mentions your evidence for that){{ pin_subpackage("myrecipe", max_pin="x.x.x") }}
(in such a case, please add a note that shortly mentions your evidence for that){{ pin_subpackage("myrecipe", max_pin=None) }}
while replacing
"myrecipe"
with eithername
if aname|lower
variable is defined in your recipe or with the lowercase name of the package in quotes.Bot commands for PR management
Please use the following BiocondaBot commands:
Everyone has access to the following BiocondaBot commands, which can be given in a comment:
@BiocondaBot please update
@BiocondaBot please add label
please review & merge
label.@BiocondaBot please fetch artifacts
You can use this to test packages locally.
Note that the
@BiocondaBot please merge
command is now depreciated. Please just squash and merge instead.Also, the bot watches for comments from non-members that include
@bioconda/<team>
and will automatically re-post them to notify the addressed<team>
.