-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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 samshee recipe #51315
Add samshee recipe #51315
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request updates the The requirements section lists dependencies for both host and runtime environments, mandating Python version 3.9 or higher, and including packages like 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: 0
🧹 Outside diff range and nitpick comments (3)
recipes/samshee/meta.yaml (3)
12-15
: Consider removing '--no-deps' and '--no-build-isolation' from the build scriptThe build section is generally well-defined:
- Using 'noarch: python' is appropriate for a pure Python package.
- Starting with build number 0 is correct for a new package.
However, the installation script uses '--no-deps' and '--no-build-isolation', which might cause issues if the package has any build-time dependencies. Consider removing these flags unless there's a specific reason for their inclusion.
Suggested change:
- script: {{ PYTHON }} -m pip install . -vv --no-deps --no-build-isolation + script: {{ PYTHON }} -m pip install . -vv
27-33
: Consider adding more comprehensive testsThe current test section is good but minimal:
- Importing the 'samshee' module checks for basic functionality.
- Running 'pip check' ensures all dependencies are correctly installed.
- Including pip in the test requirements is correct.
However, consider adding more comprehensive tests to ensure the package's functionality. This could include running the package's own test suite if available, or adding some basic command-line interface tests.
Example of additional tests:
test: imports: - samshee commands: - pip check - samshee --help # If the package has a CLI - python -m pytest --pyargs samshee # If the package includes tests requires: - pip - pytest # If adding the pytest command
35-38
: Consider adding more metadata to the about sectionThe current about section is good:
- The summary is concise and informative.
- The MIT license is correctly specified.
- Referencing the license file is good practice.
However, consider adding more metadata to provide users with more information:
Suggested additions:
about: summary: A schema-agnostic parser and writer for illumina sample sheets v2. license: MIT license_file: LICENSE home: https://github.com/username/samshee # Replace with actual URL doc_url: https://samshee.readthedocs.io/ # Replace with actual documentation URL description: | Samshee is a Python library that provides a schema-agnostic parser and writer for Illumina sample sheets version 2. It allows easy manipulation and validation of sample sheet data.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/samshee/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/samshee/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (4)
recipes/samshee/meta.yaml (4)
1-6
: LGTM: Package information is well-definedThe package name and version are correctly set using Jinja2 variables, which is a good practice for maintainability. The 'package' section properly uses these variables.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
40-43
: LGTM: Maintainers are correctly specifiedThe extra section correctly lists the recipe maintainers. This is important for package management and communication.
17-25
: LGTM: Requirements are well-definedThe host and run requirements are appropriately specified:
- Host requirements include necessary build tools.
- Run requirements match the package's dependencies.
- Version constraints are specific, which is good for reproducibility.
Please verify that the minimum versions specified match the package's actual requirements:
8-10
: LGTM: Source information is correctly specifiedThe source URL is well-constructed using Jinja2 variables, pointing to the correct PyPI package. The inclusion of a SHA256 checksum is excellent for ensuring package integrity.
Please verify the SHA256 checksum to ensure it matches the source package:
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 (3)
recipes/samshee/meta.yaml (3)
12-17
: LGTM: Build section is well-configured.The build configuration is appropriate for a pure Python package. The use of 'noarch: python' and the pip installation command with '--no-deps' and '--no-build-isolation' follow best practices. The run_exports section is correctly implemented to prevent API, ABI, and CLI breakage.
Consider adding a comment explaining the purpose of the run_exports section for future maintainers. For example:
run_exports: # Ensure compatibility with downstream recipes - {{ pin_subpackage('samshee', max_pin="x.x.x") }}
29-35
: LGTM: Test section is well-structured.The test section includes appropriate checks: an import test for 'samshee' and a 'pip check' command to verify dependency compatibility. Including pip as a test requirement is correct for running the 'pip check' command.
Consider adding a simple functionality test to ensure the package works as expected. For example:
test: imports: - samshee commands: - pip check - python -c "import samshee; assert samshee.__version__ == '0.2.1', 'Version mismatch'" requires: - pipThis additional test verifies that the imported package version matches the expected version.
37-41
: LGTM: About section provides essential information.The about section includes the necessary details: home URL, package summary, license type, and reference to the license file. This information is consistent with the package's purpose and licensing.
Consider adding a 'doc_url' field pointing to the package's documentation, if available. For example:
about: home: https://github.com/lit-regensburg/samshee summary: A schema-agnostic parser and writer for illumina sample sheets v2. license: MIT license_file: LICENSE doc_url: https://github.com/lit-regensburg/samshee#readmeThis would provide users with quick access to the package documentation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/samshee/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/samshee/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (5)
recipes/samshee/meta.yaml (5)
1-6
: LGTM: Package section is well-defined.The package name and version are correctly set using Jinja2 variables. Lowercasing the package name is consistent with Conda naming conventions. The version (0.2.1) matches the one mentioned in the PR objectives.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
43-46
: LGTM: Maintainers are correctly specified.The extra section correctly lists two recipe maintainers. It's good to see that one of the maintainers (nschcolnicov) matches the PR submitter mentioned in the PR objectives.
1-46
: Overall assessment: The recipe is well-structured and ready for inclusion.This
meta.yaml
file for the 'samshee' package adheres to Conda recipe guidelines and best practices. It includes all necessary sections (package, source, build, requirements, test, about, and extra) with appropriate details. The package version (0.2.1) and other information are consistent with the PR objectives.Key points:
- The package and source sections are correctly defined with proper versioning.
- The build section includes appropriate settings for a Python package.
- Dependencies are well-specified with version constraints.
- Test section includes basic checks, though a version check could be added.
- Licensing and package information are clearly stated.
- Maintainers are correctly listed.
Minor suggestions for improvement have been made in previous comments, but these are not blocking issues. The recipe appears ready for inclusion in the Bioconda repository.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
8-10
: LGTM: Source section is correctly defined.The source URL and SHA256 checksum are specific to this version, which is good for reproducibility. The URL structure is consistent with PyPI's file hosting service.
To verify the integrity of the source, you can run the following command:
#!/bin/bash # Verify the integrity of the source package curl -sL https://files.pythonhosted.org/packages/82/fa/895448adfc9be87b7af0a46b5c84eeae752d685966eba02d572e5c670f08/samshee-0.2.1.tar.gz | sha256sum | grep -q "e85d9363de533dab91e3190c52eda9034cbaf1e53c03f0b33f4f9b433e800b76" && echo "Checksum verified" || echo "Checksum mismatch"
19-27
: LGTM: Requirements are well-defined.The host and run requirements are appropriately specified with version constraints. The Python version requirement is consistent across environments, and the additional dependencies (jsonschema and requests) seem appropriate for the package's functionality.
To verify that these are the correct dependencies and that no dependencies are missing, you can run the following command:
This will list all import statements in the Python files, helping to ensure that all necessary dependencies are included in the requirements.
Describe your pull request here
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>
.