-
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 wsidicomizer recipe #52006
base: master
Are you sure you want to change the base?
Add wsidicomizer recipe #52006
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces a new file, 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 (
|
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: 2
🧹 Outside diff range and nitpick comments (1)
recipes/wsidicomizer/meta.yaml (1)
48-48
: Fix typo in package summary.There's a duplicate word "to" in the summary.
- summary: Tool for reading WSI files from proprietary formats and optionally convert them to to DICOM + summary: Tool for reading WSI files from proprietary formats and optionally convert them to DICOM
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
recipes/wsidicomizer/meta.yaml
(1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/wsidicomizer/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (5)
recipes/wsidicomizer/meta.yaml (5)
1-8
: LGTM! Package metadata is well-structured.
The package name, version, and source URL follow conda packaging best practices.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
9-17
: LGTM! Build configuration follows best practices.
The build configuration is well-structured with proper noarch Python package declaration and run_exports for ABI compatibility.
37-45
: LGTM! Test section is comprehensive.
Good coverage of package imports and CLI functionality testing.
51-53
: LGTM! Maintainer section is properly configured.
11-12
: Verify bioformats CLI availability.
The recipe includes an entry point for bioformats functionality, but this appears to be optional based on the constrained dependencies. Let's verify the CLI's availability.
✅ Verification successful
Let me gather more information about the package structure and dependencies to verify if bioformats is properly handled.
Let me check for any conditional imports or optional dependencies in the package configuration.
Let me check for any package metadata that might indicate optional dependencies.
Based on the analysis of the package metadata and dependencies, I can now provide a final response:
The bioformats CLI entry point is properly configured and available
The package correctly includes the bioformats CLI entry point and tests its availability in the test section. The required dependencies (scyjava and ome-types) are listed under run_constrained
requirements, which is the appropriate way to handle optional dependencies in conda recipes. The package structure with extras.bioformats
indicates this is an optional feature that will work when the constrained dependencies are present.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the bioformats CLI is conditionally imported
rg -A 5 "bioformats_cli" || echo "No direct references found"
# Check for any conditional imports of bioformats
rg -A 5 "extras.bioformats" || echo "No conditional imports found"
Length of output: 944
Script:
#!/bin/bash
# Check package dependencies and requirements
cat recipes/wsidicomizer/meta.yaml
# Check if bioformats is listed in optional dependencies
fd setup.py --exec cat {} \;
# Look for any conditional imports or optional dependencies related to bioformats
rg -l "bioformats" --type py
Length of output: 43294
Script:
#!/bin/bash
# Check for any pyproject.toml or setup.cfg that might define optional dependencies
fd "pyproject.toml|setup.cfg" --exec cat {} \;
# Look for any import statements or try-except blocks around bioformats
rg -A 5 "import.*bioformats|try:.*bioformats" --type py
Length of output: 559
Script:
#!/bin/bash
# Check for any package metadata files that might contain dependency information
cat recipes/wsidicomizer/meta.yaml
# Look for any extras or optional dependencies in the package
rg -A 5 "extras_require|optional-dependencies" || echo "No optional dependencies found"
Length of output: 4048
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/wsidicomizer/meta.yaml (3)
19-35
: Consider Python version compatibility.The recipe requires Python >=3.10, which is relatively recent. Consider if supporting Python 3.9 is feasible to increase compatibility with older systems.
Also, some dependencies have very strict version pins (e.g., wsidicom, opentile). While this is good for stability, it might need frequent updates. Consider documenting the rationale for these strict pins.
36-44
: Consider enhancing test coverage.While the basic import and help command tests are good, consider adding functional tests that verify the core conversion capabilities, perhaps with a small test file.
50-52
: Consider adding co-maintainers.For better long-term maintenance, consider adding co-maintainers from the project team or bioconda community. This helps ensure timely updates and support.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
recipes/wsidicomizer/LICENCE
(1 hunks)recipes/wsidicomizer/meta.yaml
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- recipes/wsidicomizer/LICENCE
🧰 Additional context used
🪛 yamllint
recipes/wsidicomizer/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (2)
recipes/wsidicomizer/meta.yaml (2)
1-8
: LGTM: Package metadata is well-structured.
The package name, version, and source URL are correctly specified following conda packaging best practices.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
10-12
: Verify bioformats entry point dependencies.
The recipe includes a bioformats CLI entry point, but its dependencies might be optional. Consider documenting the additional dependencies required for the bioformats functionality.
✅ Verification successful
Let me gather more information about the bioformats dependencies by checking the package's source code and requirements.
Bioformats dependencies are properly constrained
The recipe already handles bioformats dependencies correctly through the run_constrained
section, which includes scyjava
and ome-types
packages that are typically required for bioformats functionality. These optional dependencies are appropriately specified as constrained requirements, allowing users to install them only when needed for the bioformats CLI.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if bioformats dependencies are documented
rg -l "bioformats" "$(fd -t f "README.*")"
Length of output: 568
Script:
#!/bin/bash
# Check meta.yaml for bioformats dependencies
cat recipes/wsidicomizer/meta.yaml
# Check if there's a setup.py or pyproject.toml with optional dependencies
fd "setup.py|pyproject.toml" recipes/wsidicomizer/ --exec cat {}
# Look for bioformats imports and dependencies in the source
rg -l "bioformats" recipes/wsidicomizer/
Length of output: 1666
Not sure how to resolve the dependency issues, this was built from greyskull, it resolves acceptably in pip.
but it uses a pillow version that's higher than |
This package enables converting a range of formats, losslessly into DICOM files. The optional dependencies enable converting an even large range of formats lossily.
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>
.