Skip to content
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

Fixing API dependencies guards and tiledb-cloud warning #125

Merged

Conversation

ktsitsi
Copy link
Collaborator

@ktsitsi ktsitsi commented Jun 12, 2024

This PR:

  • Fixes optional dependencies introducing guards in tiledb-bioimg API.
  • Removes tiledb-cloud from the full installation. This will not affect our deployments in docker images or notebooks since the tiledb-cloud-py is already installed there. This will remove the warning - even though we were not importing the cloud-client anywhere- when a user is not logged in.
  • Adding some warnings in case optional dependencies are missing.
  • With all optional dependencies removed the user will face some warning but the tiledb.bioimg will be imported.
  • Only when the user access the a Converter that relies on a dependency not present he gets a ModuleNotFoundError
In [1]: import tiledb.bioimg
/Users/konstantinostsitsimpikos/tiledb_dev/TileDB-BioImaging/./tiledb/bioimg/converters/ome_tiff.py:12: UserWarning: OMETiff Converter requires 'tifffile' package. You can install 'tiledb-bioimg' with the 'tiff' or 'full' flag
  warnings.warn(
/Users/konstantinostsitsimpikos/tiledb_dev/TileDB-BioImaging/./tiledb/bioimg/converters/ome_zarr.py:17: UserWarning: OMEZarr Converter requires 'ome-zarr' package. You can install 'tiledb-bioimg' with the 'zarr' or 'full' flag
  warnings.warn(
/Users/konstantinostsitsimpikos/tiledb_dev/TileDB-BioImaging/./tiledb/bioimg/converters/openslide.py:25: UserWarning: Openslide Converter requires 'openslide-python' package. You can install 'tiledb-bioimg' with the 'openslide' or 'full' flag
  warnings.warn(
In [2]: import tiledb.bioimg
In [3]: from tiledb.bioimg import from_bioimg

Copy link

@jp-dark jp-dark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have one comment on a zarr import error, but other than that looks good to me.

tiledb/bioimg/converters/ome_zarr.py Outdated Show resolved Hide resolved
@ktsitsi ktsitsi force-pushed the kt/sc-48647/fix-tiledb-bioimg-dependency-import-situation branch from 528dc2b to d4367a0 Compare June 20, 2024 11:40
@ktsitsi ktsitsi merged commit 124dd41 into main Jun 20, 2024
4 checks passed
@ktsitsi ktsitsi deleted the kt/sc-48647/fix-tiledb-bioimg-dependency-import-situation branch June 20, 2024 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants