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.

Sign up for GitHub

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

Jump to bottom

Adding PNG support and enabling experimental reader in kwargs #654

Merged
merged 3 commits into from
Oct 9, 2024
Merged

Adding PNG support and enabling experimental reader in kwargs #654

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c73c403
Adding PNG support and enabling experimental reader in kwargs
ktsitsi Sep 23, 2024
f166916
PR comments reader arg
ktsitsi Sep 26, 2024
ebece9a
Make reader type an Enum
ktsitsi Oct 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions src/tiledb/cloud/bioimg/ingestion.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import logging
import warnings
from enum import Enum
from typing import Any, Iterator, Mapping, Optional, Sequence, Tuple, Union

import tiledb
Expand All @@ -15,8 +16,13 @@
DEFAULT_RESOURCES = {"cpu": "8", "memory": "4Gi"}
DEFAULT_IMG_NAME = "3.9-imaging-dev"
DEFAULT_DAG_NAME = "bioimg-ingestion"
_SUPPORTED_EXTENSIONS = (".tiff", ".tif", ".svs", ".ndpi")
_SUPPORTED_CONVERTERS = ("tiff", "zarr", "osd")
_SUPPORTED_EXTENSIONS = (".tiff", ".tif", ".svs", ".ndpi", ".png")
_SUPPORTED_CONVERTERS = ("tiff", "zarr", "osd", "png")


class ReaderType(Enum):
PRODUCTION = "production"
EXPERIMENTAL = "experimental"


def ingest(
Expand Down Expand Up @@ -74,6 +80,8 @@ def ingest(
registered in TileDB Cloud (ARN type) if ``acn`` is not set.
:param dest_config: dict configuration to pass on tiledb.VFS for the destination's
resolution
:param reader: The selected reader backend implementation either "experimental"
or "production". Default["production"]
"""

logger = get_logger_wrapper(verbose)
Expand Down Expand Up @@ -203,8 +211,10 @@ def ingest_tiff_udf(
user_converter = {
"zarr": Converters.OMEZARR,
"osd": Converters.OSD,
"png": Converters.PNG,
}.get(converter, Converters.OMETIFF)

experimental_reader = kwargs.get("experimental_reader", False)
compressor = kwargs.get("compressor", None)
if compressor:
compressor_args = dict(compressor)
Expand All @@ -229,8 +239,10 @@ def ingest_tiff_udf(
tile_scale=tile_scale,
source_config=config,
dest_config=kwargs.get("dest_config", None),
experimental_reader=experimental_reader,
**kwargs,
)

return io_uris

def register_dataset_udf(
Expand Down Expand Up @@ -350,6 +362,11 @@ def register_dataset_udf(

# serialize udf arguments
compressor = kwargs.pop("compressor", None)

# Get either the new experimental or default reader
reader = kwargs.pop("reader", ReaderType.PRODUCTION.value)
Copy link
Collaborator

@sgillies sgillies Oct 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@ktsitsi how about making reader a named keyword argument of ingest() instead?

I'm doing a deep dive into our ingestion code right now and the way we have been moving things in and out of kwargs obfuscates the code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It indeed does however, the reason I did not do it in the first place is to avoid deprecating it later and breaking the API in the next releases.

experimental_reader = reader == ReaderType.EXPERIMENTAL.value

logger.debug("Compressor: %r", compressor)
compressor_serial = serialize_filter(compressor) if compressor else None

Expand All @@ -368,6 +385,7 @@ def register_dataset_udf(
image_name=DEFAULT_IMG_NAME,
max_workers=threads,
compressor=compressor_serial,
experimental_reader=experimental_reader,
access_credentials_name=acn,
**kwargs,
)
Expand Down
Loading