From 24b57de4fad1828d4fdcda99ca630bf048b22cd6 Mon Sep 17 00:00:00 2001 From: John Kerl Date: Wed, 26 Apr 2023 23:54:00 -0400 Subject: [PATCH] [python] Add a run-time type check on contexts provided to `tiledbsoma.io` (#1297) * [python] Add a run-time type check on contexts provided to tiledbsoma.io * temp * code-review feedback --- apis/python/src/tiledbsoma/_collection.py | 3 ++- .../python/src/tiledbsoma/_common_nd_array.py | 7 ++++-- apis/python/src/tiledbsoma/_dataframe.py | 3 ++- apis/python/src/tiledbsoma/_factory.py | 3 ++- apis/python/src/tiledbsoma/_tiledb_object.py | 5 +++-- apis/python/src/tiledbsoma/io/ingest.py | 7 +++++- .../options/_soma_tiledb_context.py | 22 +++++++++++++++++++ 7 files changed, 42 insertions(+), 8 deletions(-) diff --git a/apis/python/src/tiledbsoma/_collection.py b/apis/python/src/tiledbsoma/_collection.py index fe5c1a5c87..28fdcaadfc 100644 --- a/apis/python/src/tiledbsoma/_collection.py +++ b/apis/python/src/tiledbsoma/_collection.py @@ -42,6 +42,7 @@ from ._types import OpenTimestamp from ._util import is_relative_uri, make_relative_path, uri_joinpath from .options import SOMATileDBContext +from .options._soma_tiledb_context import _validate_soma_tiledb_context # A collection can hold any sub-type of TileDBObject CollectionElementType = TypeVar("CollectionElementType", bound=AnyTileDBObject) @@ -107,7 +108,7 @@ def create( Lifecycle: Experimental. """ - context = context or SOMATileDBContext() + context = _validate_soma_tiledb_context(context) tiledb.group_create(uri=uri, ctx=context.tiledb_ctx) handle = cls._wrapper_type.open(uri, "w", context, tiledb_timestamp) cls._set_create_metadata(handle) diff --git a/apis/python/src/tiledbsoma/_common_nd_array.py b/apis/python/src/tiledbsoma/_common_nd_array.py index 6d29712a2c..0983818708 100644 --- a/apis/python/src/tiledbsoma/_common_nd_array.py +++ b/apis/python/src/tiledbsoma/_common_nd_array.py @@ -17,7 +17,10 @@ from . import _arrow_types, _util from ._tiledb_array import TileDBArray from ._types import OpenTimestamp -from .options._soma_tiledb_context import SOMATileDBContext +from .options._soma_tiledb_context import ( + SOMATileDBContext, + _validate_soma_tiledb_context, +) from .options._tiledb_create_options import TileDBCreateOptions @@ -84,7 +87,7 @@ def create( # else) sets extent to a not-power-of-two number like 999 or 1000 then the create fails if # the upper limit is exactly 2**63-1. - context = context or SOMATileDBContext() + context = _validate_soma_tiledb_context(context) schema = cls._build_tiledb_schema( type, shape, diff --git a/apis/python/src/tiledbsoma/_dataframe.py b/apis/python/src/tiledbsoma/_dataframe.py index 0f7711ff14..39cde48827 100644 --- a/apis/python/src/tiledbsoma/_dataframe.py +++ b/apis/python/src/tiledbsoma/_dataframe.py @@ -23,6 +23,7 @@ from ._tiledb_array import TileDBArray from ._types import NPFloating, NPInteger, OpenTimestamp, Slice, is_slice_of from .options import SOMATileDBContext +from .options._soma_tiledb_context import _validate_soma_tiledb_context from .options._tiledb_create_options import TileDBCreateOptions _UNBATCHED = options.BatchSize() @@ -199,7 +200,7 @@ def create( Lifecycle: Experimental. """ - context = context or SOMATileDBContext() + context = _validate_soma_tiledb_context(context) schema = _canonicalize_schema(schema, index_column_names) tdb_schema = _build_tiledb_schema( schema, diff --git a/apis/python/src/tiledbsoma/_factory.py b/apis/python/src/tiledbsoma/_factory.py index 4c79340a02..b0eb385bbf 100644 --- a/apis/python/src/tiledbsoma/_factory.py +++ b/apis/python/src/tiledbsoma/_factory.py @@ -31,6 +31,7 @@ from ._funcs import typeguard_ignore from ._types import OpenTimestamp from .options import SOMATileDBContext +from .options._soma_tiledb_context import _validate_soma_tiledb_context _Obj = TypeVar("_Obj", bound="_tiledb_object.AnyTileDBObject") _Wrapper = TypeVar("_Wrapper", bound=_tdb_handles.AnyWrapper) @@ -109,7 +110,7 @@ def open( Lifecycle: Experimental. """ - context = context or SOMATileDBContext() + context = _validate_soma_tiledb_context(context) obj = _open_internal(_tdb_handles.open, uri, mode, context, tiledb_timestamp) try: if soma_type: diff --git a/apis/python/src/tiledbsoma/_tiledb_object.py b/apis/python/src/tiledbsoma/_tiledb_object.py index 0ea871ede4..06de6a8c53 100644 --- a/apis/python/src/tiledbsoma/_tiledb_object.py +++ b/apis/python/src/tiledbsoma/_tiledb_object.py @@ -17,6 +17,7 @@ from ._types import OpenTimestamp from ._util import check_type, ms_to_datetime from .options import SOMATileDBContext +from .options._soma_tiledb_context import _validate_soma_tiledb_context _WrapperType_co = TypeVar( "_WrapperType_co", bound=_tdb_handles.AnyWrapper, covariant=True @@ -79,7 +80,7 @@ def open( Experimental. """ del platform_config # unused - context = context or SOMATileDBContext() + context = _validate_soma_tiledb_context(context) handle = cls._wrapper_type.open(uri, mode, context, tiledb_timestamp) return cls( handle, @@ -252,7 +253,7 @@ def exists( Experimental. """ check_type("uri", uri, (str,)) - context = context or SOMATileDBContext() + context = _validate_soma_tiledb_context(context) try: with cls._wrapper_type.open(uri, "r", context, tiledb_timestamp) as hdl: md_type = hdl.metadata.get(_constants.SOMA_OBJECT_TYPE_METADATA_KEY) diff --git a/apis/python/src/tiledbsoma/io/ingest.py b/apis/python/src/tiledbsoma/io/ingest.py index b6a7d82c9c..67880ca80c 100644 --- a/apis/python/src/tiledbsoma/io/ingest.py +++ b/apis/python/src/tiledbsoma/io/ingest.py @@ -56,6 +56,7 @@ from .._tiledb_object import AnyTileDBObject, TileDBObject from .._types import INGEST_MODES, IngestMode, NPNDArray, Path from ..options import SOMATileDBContext +from ..options._soma_tiledb_context import _validate_soma_tiledb_context from ..options._tiledb_create_options import TileDBCreateOptions from . import conversions @@ -118,7 +119,9 @@ def from_h5ad( ) if isinstance(input_path, ad.AnnData): - raise TypeError("Input path is an AnnData object -- did you want from_anndata?") + raise TypeError("input path is an AnnData object -- did you want from_anndata?") + + context = _validate_soma_tiledb_context(context) s = _util.get_start_stamp() logging.log_io(None, f"START Experiment.from_h5ad {input_path}") @@ -202,6 +205,8 @@ def from_anndata( "Second argument is not an AnnData object -- did you want from_h5ad?" ) + context = _validate_soma_tiledb_context(context) + # Without _at least_ an index, there is nothing to indicate the dimension indices. if anndata.obs.index.empty or anndata.var.index.empty: raise NotImplementedError("Empty AnnData.obs or AnnData.var unsupported.") diff --git a/apis/python/src/tiledbsoma/options/_soma_tiledb_context.py b/apis/python/src/tiledbsoma/options/_soma_tiledb_context.py index 0ddcab1771..1df8293f92 100644 --- a/apis/python/src/tiledbsoma/options/_soma_tiledb_context.py +++ b/apis/python/src/tiledbsoma/options/_soma_tiledb_context.py @@ -112,3 +112,25 @@ def _open_timestamp_ms(self, in_timestamp: Optional[OpenTimestamp]) -> int: if self.timestamp_ms is not None: return self.timestamp_ms return int(time.time() * 1000) + + +def _validate_soma_tiledb_context(context: Any) -> SOMATileDBContext: + """Returns the argument, as long as it's a ``SOMATileDBContext``, or a new + one if the argument is ``None``. While we already have static type-checking, + a few things are extra-important to have runtime validation on. Since it's + easy for users to pass a ``tiledb.Ctx`` when a ``SOMATileDBContext`` is + expected, we should offer a helpful redirect when they do. + """ + + if context is None: + return SOMATileDBContext() + + if isinstance(context, tiledb.Ctx): + raise TypeError( + "context is a tiledb.Ctx, not a SOMATileDBContext -- please wrap it in tiledbsoma.SOMATileDBContext(...)" + ) + + if not isinstance(context, SOMATileDBContext): + raise TypeError("context is not a SOMATileDBContext") + + return context