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

[python] TileDBConfig type alias, misc. type annotations #3317

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions apis/python/src/tiledbsoma/_indexer.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@


def tiledbsoma_build_index(
data: IndexerDataType, *, context: Optional["SOMATileDBContext"] = None
data: IndexerDataType, *, context: Optional[SOMATileDBContext] = None
) -> IndexLike:
"""Initialize re-indexer for provided indices (deprecated).

Expand All @@ -54,7 +54,7 @@ class IntIndexer:
"""

def __init__(
self, data: IndexerDataType, *, context: Optional["SOMATileDBContext"] = None
self, data: IndexerDataType, *, context: Optional[SOMATileDBContext] = None
):
"""Initialize re-indexer for provided indices.

Expand Down
40 changes: 18 additions & 22 deletions apis/python/src/tiledbsoma/experimental/ingest.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import warnings
from pathlib import Path
from typing import (
TYPE_CHECKING,
List,
Optional,
Sequence,
Expand All @@ -38,6 +37,7 @@


from somacore import Axis, CoordinateSpace, IdentityTransform, ScaleTransform
from somacore.options import PlatformConfig

from .. import (
Collection,
Expand All @@ -61,6 +61,8 @@
from .._soma_object import AnySOMAObject
from .._types import IngestMode
from ..io import from_anndata
from ..io._common import AdditionalMetadata
from ..io._registration import ExperimentAmbientLabelMapping
from ..io.ingest import (
IngestCtx,
IngestionParams,
Expand All @@ -69,19 +71,13 @@
_write_arrow_table,
add_metadata,
)
from ..options._soma_tiledb_context import SOMATileDBContext
from ..options._tiledb_create_write_options import (
TileDBCreateOptions,
TileDBWriteOptions,
)
from ._util import _read_visium_software_version

if TYPE_CHECKING:
from somacore.options import PlatformConfig

from ..io._common import AdditionalMetadata
from ..io._registration import ExperimentAmbientLabelMapping
from ..options import SOMATileDBContext


def path_validator(instance, attribute, value: Path) -> None: # type: ignore[no-untyped-def]
if not value.exists():
Expand Down Expand Up @@ -273,8 +269,8 @@ def from_visium(
measurement_name: str,
scene_name: str,
*,
context: Optional["SOMATileDBContext"] = None,
platform_config: Optional["PlatformConfig"] = None,
context: Optional[SOMATileDBContext] = None,
platform_config: Optional[PlatformConfig] = None,
obs_id_name: str = "obs_id",
var_id_name: str = "var_id",
X_layer_name: str = "data",
Expand All @@ -286,7 +282,7 @@ def from_visium(
X_kind: Union[Type[SparseNDArray], Type[DenseNDArray]] = SparseNDArray,
registration_mapping: Optional["ExperimentAmbientLabelMapping"] = None,
uns_keys: Optional[Sequence[str]] = None,
additional_metadata: "AdditionalMetadata" = None,
additional_metadata: AdditionalMetadata = None,
use_raw_counts: bool = False,
write_obs_spatial_presence: bool = False,
write_var_spatial_presence: bool = False,
Expand Down Expand Up @@ -625,9 +621,9 @@ def _write_scene_presence_dataframe(
df_uri: str,
*,
ingestion_params: IngestionParams,
additional_metadata: "AdditionalMetadata" = None,
platform_config: Optional["PlatformConfig"] = None,
context: Optional["SOMATileDBContext"] = None,
additional_metadata: AdditionalMetadata = None,
platform_config: Optional[PlatformConfig] = None,
context: Optional[SOMATileDBContext] = None,
) -> DataFrame:
s = _util.get_start_stamp()

Expand Down Expand Up @@ -690,9 +686,9 @@ def _write_visium_spots(
id_column_name: str,
*,
ingestion_params: IngestionParams,
additional_metadata: "AdditionalMetadata" = None,
platform_config: Optional["PlatformConfig"] = None,
context: Optional["SOMATileDBContext"] = None,
additional_metadata: AdditionalMetadata = None,
platform_config: Optional[PlatformConfig] = None,
context: Optional[SOMATileDBContext] = None,
) -> PointCloudDataFrame:
"""Creates, opens, and writes data to a ``PointCloudDataFrame`` with the spot
locations and metadata. Returns the open dataframe for writing.
Expand Down Expand Up @@ -754,8 +750,8 @@ def _create_or_open_scene(
uri: str,
*,
ingestion_params: IngestionParams,
context: Optional["SOMATileDBContext"],
additional_metadata: "AdditionalMetadata" = None,
context: Optional[SOMATileDBContext],
additional_metadata: AdditionalMetadata = None,
) -> Scene:
"""Creates or opens a ``Scene`` and returns it open for writing."""
try:
Expand All @@ -777,9 +773,9 @@ def _create_visium_tissue_images(
image_paths: List[Tuple[str, Path, Optional[float]]],
*,
image_channel_first: bool,
additional_metadata: "AdditionalMetadata" = None,
platform_config: Optional["PlatformConfig"] = None,
context: Optional["SOMATileDBContext"] = None,
additional_metadata: AdditionalMetadata = None,
platform_config: Optional[PlatformConfig] = None,
context: Optional[SOMATileDBContext] = None,
ingestion_params: IngestionParams,
use_relative_uri: Optional[bool] = None,
) -> MultiscaleImage:
Expand Down
2 changes: 1 addition & 1 deletion apis/python/src/tiledbsoma/io/outgest.py
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ def load_varp(measurement: Measurement, key: str, nvar: int) -> sp.csr_matrix:
varm = _resolve_futures(varm)
obsp = _resolve_futures(obsp)
varp = _resolve_futures(varp)
anndata_X = anndata_X_future.result() if anndata_X_future is not None else None
anndata_X = anndata_X_future.result() if anndata_X_future else None
uns: UnsDict = (
_resolve_futures(uns_future.result(), deep=True)
if uns_future is not None
Expand Down
3 changes: 2 additions & 1 deletion apis/python/src/tiledbsoma/options/__init__.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
from ._soma_tiledb_context import SOMATileDBContext
from ._soma_tiledb_context import SOMATileDBContext, TileDBConfig
from ._tiledb_create_write_options import TileDBCreateOptions, TileDBWriteOptions

__all__ = [
"SOMATileDBContext",
"TileDBConfig",
"TileDBCreateOptions",
"TileDBWriteOptions",
]
46 changes: 22 additions & 24 deletions apis/python/src/tiledbsoma/options/_soma_tiledb_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import threading
import time
from concurrent.futures import ThreadPoolExecutor
from typing import Any, Dict, Literal, Mapping, Optional, Union
from typing import Any, Dict, Literal, Optional, Union

from somacore import ContextBase
from typing_extensions import Self
Expand All @@ -19,18 +19,21 @@
from .._types import OpenTimestamp
from .._util import ms_to_datetime, to_timestamp_ms

_Unset = Literal["__unset__"]
_UNSET: _Unset = "__unset__"

TileDBConfig = Dict[str, Union[str, float]]
ReplaceConfig = Dict[str, Union[str, float, None]]
"""Replacing a value with ``None`` serves to delete that key."""

def _default_config(
override: Mapping[str, Union[str, float]]
) -> Dict[str, Union[str, float]]:

def _default_config(override: TileDBConfig) -> TileDBConfig:
"""Returns a fresh dictionary with TileDB config values.

These should be reasonable defaults that can be used out-of-the-box.
``override`` does exactly what it says: overrides default entries.
"""
cfg: Dict[str, Union[str, float]] = {
"sm.mem.reader.sparse_global_order.ratio_array_data": 0.3
}
cfg: TileDBConfig = {"sm.mem.reader.sparse_global_order.ratio_array_data": 0.3}
cfg.update(override)
return cfg

Expand All @@ -47,10 +50,6 @@ def _maybe_timestamp_ms(input: Optional[OpenTimestamp]) -> Optional[int]:
return to_timestamp_ms(input)


_Unset = Literal["__unset__"]
_UNSET: _Unset = "__unset__"


class SOMATileDBContext(ContextBase):
"""Maintains TileDB-specific context for TileDB-SOMA objects.
This context can be shared across multiple objects,
Expand All @@ -66,7 +65,7 @@ class SOMATileDBContext(ContextBase):

def __init__(
self,
tiledb_config: Optional[Dict[str, Union[str, float]]] = None,
tiledb_config: Optional[TileDBConfig] = None,
timestamp: Optional[OpenTimestamp] = None,
threadpool: Optional[ThreadPoolExecutor] = None,
) -> None:
Expand All @@ -80,9 +79,6 @@ def __init__(
it is used to construct a new ``Ctx``.

Args:
tiledb_ctx: An existing TileDB Context for use as the TileDB Context
in this configuration.

tiledb_config: A set of TileDB configuration options to use,
overriding the default configuration.

Expand Down Expand Up @@ -119,7 +115,7 @@ def __init__(
"""
self._lock = threading.Lock()
"""A lock to ensure single initialization of ``_tiledb_ctx``."""
self._initial_config: Optional[Dict[str, Union[str, float]]] = (
self._initial_config: Optional[TileDBConfig] = (
None if tiledb_config is None else _default_config(tiledb_config)
)

Expand Down Expand Up @@ -163,7 +159,7 @@ def native_context(self) -> clib.SOMAContext:
return self._native_context

@property
def tiledb_config(self) -> Dict[str, Union[str, float]]:
def tiledb_config(self) -> TileDBConfig:
"""The TileDB configuration dictionary for this SOMA context.

If this ``SOMATileDBContext`` already has a ``tiledb_ctx``, this will
Expand All @@ -177,7 +173,7 @@ def tiledb_config(self) -> Dict[str, Union[str, float]]:
with self._lock:
return self._internal_tiledb_config()

def _internal_tiledb_config(self) -> Dict[str, Union[str, float]]:
def _internal_tiledb_config(self) -> TileDBConfig:
"""Internal function for getting the TileDB Config.

Returns a new dict with the contents. Caller must hold ``_lock``.
Expand All @@ -197,7 +193,7 @@ def _internal_tiledb_config(self) -> Dict[str, Union[str, float]]:
def replace(
self,
*,
tiledb_config: Optional[Dict[str, Any]] = None,
tiledb_config: Optional[ReplaceConfig] = None,
timestamp: Union[None, OpenTimestamp, _Unset] = _UNSET,
threadpool: Union[None, ThreadPoolExecutor, _Unset] = _UNSET,
) -> Self:
Expand All @@ -208,8 +204,6 @@ def replace(
A dictionary of parameters for `tiledb.Config() <https://tiledb-inc-tiledb.readthedocs-hosted.com/projects/tiledb-py/en/stable/python-api.html#config>`_.
To remove a parameter from the existing config, provide ``None``
as the value.
tiledb_ctx:
A TileDB Context to replace the current context with.
timestamp:
A timestamp to replace the current timestamp with.
Explicitly passing ``None`` will remove the timestamp.
Expand All @@ -230,9 +224,13 @@ def replace(
"""
with self._lock:
if tiledb_config is not None:
new_config = self._internal_tiledb_config()
new_config: ReplaceConfig = dict(self._internal_tiledb_config())
new_config.update(tiledb_config)
tiledb_config = {k: v for (k, v) in new_config.items() if v is not None}
new_tiledb_config = {
k: v for k, v in new_config.items() if v is not None
}
else:
new_tiledb_config = None

if timestamp == _UNSET:
# Keep the existing timestamp if not overridden.
Expand All @@ -243,7 +241,7 @@ def replace(

assert timestamp is None or isinstance(timestamp, (datetime.datetime, int))
return type(self)(
tiledb_config=tiledb_config,
tiledb_config=new_tiledb_config,
timestamp=timestamp,
threadpool=threadpool,
)
Expand Down
30 changes: 15 additions & 15 deletions apis/python/tests/test_experiment_query.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import re
from concurrent import futures
from contextlib import nullcontext
from typing import Tuple
from typing import Optional, Sequence, Tuple
from unittest import mock

import numpy as np
Expand All @@ -10,10 +10,10 @@
import pytest
from pyarrow import ArrowInvalid
from scipy import sparse
from somacore import AxisQuery, options
from somacore import AxisQuery, ExperimentAxisQuery, options

import tiledbsoma as soma
from tiledbsoma import SOMATileDBContext, _factory
from tiledbsoma import Experiment, SOMATileDBContext
from tiledbsoma._collection import CollectionBase
from tiledbsoma.experiment_query import X_as_series

Expand All @@ -24,27 +24,27 @@


@pytest.fixture
def X_layer_names():
def X_layer_names() -> Optional[Sequence[str]]:
return ["raw"]


@pytest.fixture
def obsp_layer_names():
def obsp_layer_names() -> Optional[Sequence[str]]:
return None


@pytest.fixture
def varp_layer_names():
def varp_layer_names() -> Optional[Sequence[str]]:
return None


@pytest.fixture
def obsm_layer_names():
def obsm_layer_names() -> Optional[Sequence[str]]:
return None


@pytest.fixture
def varm_layer_names():
def varm_layer_names() -> Optional[Sequence[str]]:
return None


Expand All @@ -58,7 +58,7 @@ def soma_experiment(
varp_layer_names,
obsm_layer_names,
varm_layer_names,
):
) -> Experiment:
with soma.Experiment.create((tmp_path / "exp").as_posix()) as exp:
add_dataframe(exp, "obs", n_obs)
ms = exp.add_new_collection("ms")
Expand Down Expand Up @@ -88,18 +88,18 @@ def soma_experiment(
for varm_layer_name in varm_layer_names:
add_sparse_array(varm, varm_layer_name, (n_vars, N_FEATURES))

return _factory.open((tmp_path / "exp").as_posix())
return Experiment.open((tmp_path / "exp").as_posix())


def get_soma_experiment_with_context(soma_experiment, context):
def get_soma_experiment_with_context(soma_experiment, context) -> Experiment:
soma_experiment.close()
return _factory.open(soma_experiment.uri, context=context)
return Experiment.open(soma_experiment.uri, context=context)


@pytest.mark.parametrize("n_obs,n_vars,X_layer_names", [(101, 11, ("raw", "extra"))])
def test_experiment_query_all(soma_experiment):
"""Test a query with default obs_query / var_query -- i.e., query all."""
with soma.ExperimentAxisQuery(soma_experiment, "RNA") as query:
with ExperimentAxisQuery(soma_experiment, "RNA") as query:
assert query.n_obs == 101
assert query.n_vars == 11

Expand Down Expand Up @@ -134,8 +134,8 @@ def test_experiment_query_all(soma_experiment):
ad = query.to_anndata("raw")
assert ad.n_obs == query.n_obs and ad.n_vars == query.n_vars

assert set(ad.obs.keys().to_list()) == set(["soma_joinid", "label"])
assert set(ad.var.keys().to_list()) == set(["soma_joinid", "label"])
assert set(ad.obs.keys()) == {"soma_joinid", "label"}
assert set(ad.var.keys()) == {"soma_joinid", "label"}

obs = soma_experiment.obs.read().concat().to_pandas()
obs.index = obs.index.map(str)
Expand Down