Update Crabs #51394
Update Crabs #51394
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces several modifications to the Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
recipes/crabs/meta.yaml (1)
37-38: LGTM! Consider documenting version constraints.The addition of specific versions for
wgetandmusclein the run requirements is good for ensuring reproducibility.It might be helpful to add a comment explaining why these specific versions are required. This can aid future maintenance and upgrades of the package.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/crabs/meta.yaml (2 hunks)
🧰 Additional context used
🪛 yamllint
recipes/crabs/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (5)
recipes/crabs/meta.yaml (5)
1-7: LGTM! Consider documenting the significant version bump.The use of Jinja2 templating for package name and version is a good practice. The addition of the SHA256 checksum enhances security. However, the version bump from 0.1.1 to 1.0.3 seems significant.
Could you please verify if this version bump aligns with the upstream changes? If there are breaking changes, consider documenting them in the package description or README.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
10-11: LGTM! Improved source URL and added checksum.The updated source URL using the version variable is more maintainable. The inclusion of the SHA256 checksum enhances security and integrity verification.
15-15: LGTM! Improved build script configuration.The addition of the
--no-depsflag is appropriate for Conda recipes, as it prevents pip from installing dependencies (which should be managed by Conda). The increased verbosity (-vvv) can be helpful for troubleshooting installation issues if they arise.
22-28: Please clarify the status of commented-out host dependencies.Several dependencies in the host section have been commented out. If these are no longer needed for building the package, it would be cleaner to remove them entirely rather than leaving them commented. If they are still necessary, they should be uncommented.
Could you please clarify the reasoning behind commenting out these dependencies? If they are truly not needed, consider removing them completely for better maintainability.
1-3: Note on static analysis warning.The static analysis tool (yamllint) reported a syntax error for the '%' character at the beginning of the file. This is a false positive. The '%' character is valid in this context as it's part of the Jinja2 templating syntax used in Conda recipes.
No action is needed to address this warning.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
| @@ -33,8 +36,8 @@ requirements: | |||
| - numpy | |||
| - pandas >=0.23.4 | |||
| - matplotlib-base | |||
| - wget | |||
| - muscle >=3.8.31 | |||
| - wget =1.21.3 | |||
There was a problem hiding this comment.
not sure - just copied that from the other PR. I think we can go for the latest version here.
There was a problem hiding this comment.
I guess any new semver compatible version should be OK.
Pinning with exact versions is usually not recommended.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
recipes/crabs/meta.yaml (1)
15-18: Consider adjusting therun_exportssectionThe build script changes look good:
- Adding
--no-depsensures stricter control over dependencies.- Increased verbosity (
-vvv) can help with debugging.However, for the
run_exportssection, consider the following suggestion from a previous review:noarch: python run_exports: - {{ pin_subpackage(name, max_pin="x.x") }}This change would make the
noarch: pythondeclaration part of thebuildsection and adjust the indentation ofrun_exports.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/crabs/meta.yaml (2 hunks)
🧰 Additional context used
🪛 yamllint
recipes/crabs/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (5)
recipes/crabs/meta.yaml (5)
1-7: LGTM: Improved package metadata with Jinja2 templatingThe introduction of Jinja2 variables for name, version, and sha256 enhances maintainability. The package version update to 1.0.3 aligns with the PR objectives, and the dynamic package name ensures consistency.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
10-11: LGTM: Improved source specificationThe source URL now dynamically incorporates the version, facilitating easier future updates. The addition of the SHA256 checksum enhances the security and integrity of the package source.
24-30: Verify the removal of host requirementsSeveral dependencies have been commented out in the host requirements section. Could you please confirm if these dependencies are no longer needed for building the package? If they're truly not required, consider removing the commented lines entirely for cleaner code.
39-40: LGTM: Improved version pinning for run requirementsThe addition of specific versions for
wgetandmusclein the run requirements is a good practice. This helps ensure reproducibility and aligns with best practices for dependency management in Conda recipes.
1-1: Note on yamllint warningThe yamllint tool reports a syntax error for the '%' character. This is a false positive as the '%' is part of valid Jinja2 syntax, which is supported in Conda meta.yaml files. We can safely ignore this warning.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
Related to: gjeunen/reference_database_creator#66
Also includes changes from: #35250 (partly)
Please read the guidelines for Bioconda recipes before opening a pull request (PR).
General instructions
@BiocondaBot please add labelcommand.@bioconda/corein 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_exportssection 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 eithernameif aname|lowervariable 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 labelplease review & mergelabel.@BiocondaBot please fetch artifactsYou can use this to test packages locally.
Note that the
@BiocondaBot please mergecommand 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>.