-
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
Update decom recipe to version 0.0.32 and align dependencies #51723
Conversation
Warning Rate limit exceeded@mencian has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 11 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces modifications to the Possibly related PRs
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 (
|
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/decom/meta.yaml (1)
10-11
: Remove redundant SHA256 commentThe comment "Update this if necessary" is redundant since SHA256 checksums must always match the source package exactly. Consider removing this comment to avoid confusion.
- sha256: c9efaa97b7abfe6f19f01a199ca6c7a7b32ec699f886d226667b8b5fe562dd88 # Update this if necessary + sha256: c9efaa97b7abfe6f19f01a199ca6c7a7b32ec699f886d226667b8b5fe562dd88
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/decom/meta.yaml (3 hunks)
🧰 Additional context used
🪛 yamllint
recipes/decom/meta.yaml
[error] 63-63: trailing spaces
(trailing-spaces)
[error] 64-64: trailing spaces
(trailing-spaces)
🔇 Additional comments (5)
recipes/decom/meta.yaml (5)
5-6
: LGTM: Package naming follows best practicesThe change to lowercase package naming using Jinja templating aligns with Python packaging conventions.
Line range hint
13-22
: LGTM: Build configuration is correctThe build number increment is appropriate for the dependency updates, and entry points are properly configured.
40-48
: LGTM: Comprehensive test configurationThe test configuration is thorough, covering all major modules and command-line interfaces.
61-64
:⚠️ Potential issueAdd missing metadata fields
Several important metadata fields are empty:
license_file
should point to the LICENSE file in the packagedoc_url
anddev_url
should be populated to improve package discoverabilityThe previous values were:
- doc_url: "https://github.com/CamilaDuitama/decOM"
- dev_url: "https://camiladuitama.github.io/gradfolio/"
Please verify if these URLs are still valid and restore them if appropriate.
✅ Verification successful
Add LICENSE file path and restore documentation URLs
The verification shows:
- A LICENSE file exists at the repository root that can be referenced
- Both documentation URLs are still valid and accessible:
Please update the metadata fields:
license_file: LICENSE doc_url: https://github.com/CamilaDuitama/decOM dev_url: https://camiladuitama.github.io/gradfolio/🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if LICENSE file exists in the package fd -t f -i "license" # Verify if the documentation URLs are still valid curl -I https://github.com/CamilaDuitama/decOM curl -I https://camiladuitama.github.io/gradfolio/Length of output: 14238
🧰 Tools
🪛 yamllint
[error] 63-63: trailing spaces
(trailing-spaces)
[error] 64-64: trailing spaces
(trailing-spaces)
27-36
: Consider more flexible version constraintsThe current version constraints might be overly restrictive:
- Limiting Python to 3.12 only might unnecessarily exclude compatible Python versions
- Very tight version constraints (e.g., pandas >=2.2.3,<2.3) could cause dependency resolution issues
Consider:
- Supporting a broader Python range (e.g., >=3.9)
- Using more flexible version constraints unless there are known compatibility issues
@BiocondaBot please add label |
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/decom/meta.yaml (2)
68-69
: Consider adding documentation and development URLs.The
doc_url
anddev_url
fields are empty. Consider adding:
doc_url
pointing to the GitHub repository (since it contains documentation)dev_url
pointing to the GitHub repository for development references- doc_url: - dev_url: + doc_url: https://github.com/CamilaDuitama/decOM + dev_url: https://github.com/CamilaDuitama/decOM🧰 Tools
🪛 yamllint
[error] 68-68: trailing spaces
(trailing-spaces)
[error] 69-69: trailing spaces
(trailing-spaces)
65-67
: Fix trailing spaces in description.Remove trailing spaces from the description field to resolve yamllint warnings.
- decOM is a similarity-based microbial source tracking tool for - contamination assessment of ancient oral samples using k-mer-based methods. + decOM is a similarity-based microbial source tracking tool for + contamination assessment of ancient oral samples using k-mer-based methods.🧰 Tools
🪛 yamllint
[error] 66-66: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/decom/meta.yaml (3 hunks)
🧰 Additional context used
🪛 yamllint
recipes/decom/meta.yaml
[error] 66-66: trailing spaces
(trailing-spaces)
[error] 68-68: trailing spaces
(trailing-spaces)
[error] 69-69: trailing spaces
(trailing-spaces)
🔇 Additional comments (4)
recipes/decom/meta.yaml (4)
5-5
: LGTM: Package naming follows Conda best practices.The change to lowercase package name using Jinja template is correct and follows Conda packaging conventions.
42-50
: LGTM: Comprehensive test imports.The test imports cover all major submodules, providing good coverage for package integrity verification.
29-38
:⚠️ Potential issueEnsure consistent dependency version constraints.
A few observations about the dependency specifications:
- The Python requirement (>=3.12,<3.13) is very restrictive. Consider if this is intentional as it limits the package to only Python 3.12.
- Some dependencies have upper bounds (numpy, pandas, plotly) while others (dask, importlib_resources) don't. Consider adding upper bounds for consistency.
Let's verify if other packages support this Python version:
#!/bin/bash # Check Python version constraints in dependencies echo "Checking dask Python requirements:" rg -A 2 'name: "dask"' . | rg 'python' echo "Checking pandas Python requirements:" rg -A 2 'name: "pandas"' . | rg 'python'
23-24
: 🛠️ Refactor suggestionConsider relaxing the version pinning strategy.
The current
max_pin='x.x.x'
is very restrictive, requiring an exact version match. For a package at version 0.0.32 (pre-1.0), consider using a more relaxed pinning strategy likemax_pin='x.x'
to allow patch-level updates, unless there are specific ABI compatibility concerns.
recipes/decom/meta.yaml
Outdated
summary: "decOM: Similarity-based microbial source tracking for contamination assessment of ancient oral samples using k-mer-based methods" | ||
doc_url: "https://github.com/CamilaDuitama/decOM" | ||
dev_url: "https://camiladuitama.github.io/gradfolio/" | ||
license_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.
Add the license file specification.
The license_file
field is empty but the package is MIT licensed. This field should point to the LICENSE file in the source distribution.
Please add:
- license_file:
+ license_file: LICENSE
📝 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.
license_file: | |
license_file: LICENSE |
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.
Please add a license 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.
@bgruening, thank you for pointing that out. Please ensure that the LICENSE
file is included in the source distribution and update the license_file
field in meta.yaml
accordingly.
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.
The PyPI distribution includes the LICENSE file. I added a pointer to the meta.yaml file; I hope this fixes the issue.
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.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
recipes/decom/meta.yaml
Outdated
- pandas | ||
- plotly >=5.5.0 | ||
- python | ||
- python >=3.12,<3.13 |
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.
Why are so strict pinning needed?
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 removed the strict pinning
74b3a32
to
db34062
Compare
db34062
to
4261ec5
Compare
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/decom/meta.yaml (1)
69-70
: Consider separating documentation and development URLs
Both doc_url
and dev_url
point to the same GitHub repository. Consider:
- Creating dedicated documentation (e.g., using GitHub Pages, ReadTheDocs)
- Using the documentation URL for
doc_url
and keeping the GitHub repository fordev_url
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/decom/meta.yaml (3 hunks)
🔇 Additional comments (4)
recipes/decom/meta.yaml (4)
Line range hint 13-24
: LGTM: Build configuration follows best practices
The build configuration is well structured with:
- Appropriate build number increment
- Correct entry points configuration
- Good pinning strategy in run_exports
44-52
: LGTM: Comprehensive test coverage
The test section provides good coverage by:
- Testing imports of all main modules
- Including command-line interface tests
29-40
: Consider relaxing version constraints
The current version constraints might be overly restrictive:
- Python 3.12-3.14 is a very narrow range and 3.12 is quite recent
- Dependencies like numpy, pandas, and dask are pinned to very recent versions
This could limit the package's compatibility with other tools in the bioconda ecosystem. Consider:
- Supporting Python 3.8+ unless there's a specific need for 3.12 features
- Relaxing dependency version upper bounds unless there are known compatibility issues
#!/bin/bash
# Check other bioconda packages' Python compatibility
rg "python.*3\.12" | grep "run:"
9-11
: Verify the SHA256 checksum for version 0.0.32
Please verify that the SHA256 checksum matches the source tarball for version 0.0.32.
@BiocondaBot please add label |
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>
.