Update decom: add kmtricks dependancy #51386
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request modifies the Possibly related PRs
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 (3)
recipes/decom/meta.yaml (3)
Line range hint
18-24: New entry points look good, consider adding a comment for clarity.The addition of new entry points for various decOM subcommands is well-structured and consistent. This change improves the package's usability by allowing direct access to different functionalities from the command line.
Consider adding a brief comment above the entry_points section to explain the purpose of these commands, for example:
# Entry points for decOM main command and subcommands entry_points: - decOM = decOM.__main__:main # ... (rest of the entry points)
40-40: New kmtricks dependency added, consider specifying a version.The addition of 'kmtricks' as a runtime requirement aligns with the PR objectives. However, to ensure long-term compatibility and reproducibility, it's recommended to specify a version or version range for this dependency.
Consider updating the requirement to include a version specification, for example:
- kmtricks >=1.0.0,<2.0.0Replace
1.0.0with the minimum compatible version and adjust the upper bound as needed. This helps prevent potential issues with future incompatible versions.
Line range hint
1-67: Overall changes look good, consider updating package version in the future.The modifications to the meta.yaml file, including the build number increment, addition of new entry points, and the new kmtricks dependency, are well-implemented and align with the PR objectives. These changes improve the package's functionality and usability.
If these changes significantly alter the package's functionality or API, consider updating the package version (currently 0.0.32) in a future PR. This would follow semantic versioning principles and clearly communicate the extent of changes to users.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/decom/meta.yaml (2 hunks)
🧰 Additional context used
🔇 Additional comments (1)
recipes/decom/meta.yaml (1)
15-15: Build number increment looks good.The build number has been correctly incremented from 0 to 1. This is appropriate when making changes to a package without changing its version.
martin-g
force-pushed
the
update_decom
branch
from
5453264 to
fb22ae2
Compare
|
Hey @martin-g, |
|
I doubt! |
martin-g
force-pushed
the
update_decom
branch
from
fb22ae2 to
eb3b079
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
recipes/decom/meta.yaml (2)
Line range hint
18-23: LGTM: New entry points added correctly.The addition of new entry points for various decOM modules (aOralOut, format, LOO, CV, MST) is correct and aligns with the PR objectives. Each entry point properly maps to the
__main__:mainfunction of its respective module.Consider adding a blank line before and after the entry_points section for improved readability:
noarch: python entry_points: - decOM = decOM.__main__:main - decOM-aOralOut = decOM.aOralOut.__main__:main - decOM-format = decOM.format.__main__:main - decOM-LOO = decOM.LOO.__main__:main - decOM-CV = decOM.CV.__main__:main - decOM-MST = decOM.MST.__main__:main script: "{{ PYTHON }} -m pip install . -vv"
32-32: LGTM with suggestions: Dependencies updated correctly.The update of dask to version >= 2024.5.0 and the addition of kmtricks as a new dependency are correct and align with the PR objectives.
Consider the following suggestions:
- For the dask update, it might be beneficial to use the
{{ pin_compatible('dask', max_pin='x.x') }}syntax to ensure compatibility with future versions while avoiding potential breaking changes:- {{ pin_compatible('dask', min_pin='x.x', max_pin='x.x') }} >= 2024.5.0
- For the new kmtricks dependency, consider adding a version constraint and using
pin_compatible:- {{ pin_compatible('kmtricks', max_pin='x.x') }}
- As mentioned in the PR objectives, consider adding a
run_exportssection for kmtricks to help manage dependency versions and prevent downstream recipe issues:build: run_exports: - {{ pin_subpackage('kmtricks', max_pin='x.x') }}These changes will help ensure better version management and compatibility across the ecosystem.
Also applies to: 40-40
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/decom/meta.yaml (2 hunks)
🧰 Additional context used
🔇 Additional comments (2)
recipes/decom/meta.yaml (2)
15-15: LGTM: Build number increment is correct.Incrementing the build number from 0 to 1 is appropriate when making changes to the package without updating its version. This follows Conda packaging best practices.
Line range hint
1-65: Overall assessment: Changes are appropriate and align with PR objectives.The modifications to the
meta.yamlfile for the decOM package are well-implemented and achieve the stated objectives of the PR. The build number increment, addition of new entry points, and updates to dependencies (particularly the addition of kmtricks) are all correct and necessary changes.A few minor suggestions have been provided to further improve version management and compatibility:
- Adding blank lines around the
entry_pointssection for better readability.- Using
pin_compatiblefor the dask dependency.- Adding version constraints and using
pin_compatiblefor the new kmtricks dependency.- Considering the addition of a
run_exportssection for kmtricks.These suggestions, if implemented, would enhance the robustness of the package configuration. Overall, the changes in this PR are approved and ready for merging after addressing the minor suggestions.
|
Closing in favor of #51911. |
After talking to the original author of decOM (@CamilaDuitama), this PR adds kmtricks dependancy to decOM.
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>.