From b5f6917538f7632cb5d7e6d4518e199b644d5f3a Mon Sep 17 00:00:00 2001 From: Ryan Williams Date: Mon, 25 Mar 2024 11:08:33 -0400 Subject: [PATCH 1/3] use requirements_dev.txt in CI, setup.py --- .github/workflows/python-ci-single.yml | 5 +---- .github/workflows/r-python-interop-testing.yml | 5 +---- apis/python/requirements_dev.txt | 8 +++++--- apis/python/setup.py | 7 +------ 4 files changed, 8 insertions(+), 17 deletions(-) diff --git a/.github/workflows/python-ci-single.yml b/.github/workflows/python-ci-single.yml index fe0bda6067..01974aa571 100644 --- a/.github/workflows/python-ci-single.yml +++ b/.github/workflows/python-ci-single.yml @@ -100,11 +100,8 @@ jobs: # dist # key: libtiledbsoma-build-dist-${{ inputs.os }}-${{ inputs.python_version }}-${{ hashFiles('libtiledbsoma', 'scripts/bld') }} - - name: Install testing prereqs - run: python -m pip -v install -U pip pytest-cov 'typeguard==4.1.5' types-setuptools sparse - - name: Install tiledbsoma - run: python -m pip -v install -e apis/python + run: python -m pip -v install -e apis/python[dev] env: CC: ${{ inputs.cc }} CXX: ${{ inputs.cxx }} diff --git a/.github/workflows/r-python-interop-testing.yml b/.github/workflows/r-python-interop-testing.yml index 8991e5caf3..eae6f18b62 100644 --- a/.github/workflows/r-python-interop-testing.yml +++ b/.github/workflows/r-python-interop-testing.yml @@ -69,11 +69,8 @@ jobs: cache: pip cache-dependency-path: ./apis/python/setup.py - - name: Install testing prereqs - run: python -m pip -v install -U pip pytest-cov 'typeguard==4.1.5' types-setuptools - - name: Install tiledbsoma - run: python -m pip -v install -e apis/python + run: python -m pip -v install -e apis/python[dev] - name: Show Python package versions run: | diff --git a/apis/python/requirements_dev.txt b/apis/python/requirements_dev.txt index a3bfa08be9..f1b06213a8 100644 --- a/apis/python/requirements_dev.txt +++ b/apis/python/requirements_dev.txt @@ -1,5 +1,7 @@ -cmake >= 3.21 -pybind11-global >= 2.10.0 -typeguard==4.1.5 +black +ruff pytest +pytest-cov sparse +typeguard==4.1.5 +types-setuptools diff --git a/apis/python/setup.py b/apis/python/setup.py index 55cf4e5c15..bc3cf4818e 100644 --- a/apis/python/setup.py +++ b/apis/python/setup.py @@ -332,12 +332,7 @@ def run(self): "typing-extensions", # Note "-" even though `import typing_extensions` ], extras_require={ - "dev": [ - "black", - "ruff", - "pytest", - "typeguard==4.1.5", - ] + "dev": open("requirements_dev.txt").read(), }, python_requires=">=3.8", cmdclass={"build_ext": build_ext, "bdist_wheel": bdist_wheel}, From 8a065932a6d045c6c819487da2ce7b0d563280eb Mon Sep 17 00:00:00 2001 From: Ryan Williams Date: Mon, 25 Mar 2024 11:09:22 -0400 Subject: [PATCH 2/3] use typeguard>=4.2, fixup some type checks https://github.com/agronholm/typeguard/issues/442 was fixed in typeguard 4.2.0 --- apis/python/requirements_dev.txt | 2 +- apis/python/src/tiledbsoma/_collection.py | 5 ++-- apis/python/src/tiledbsoma/_factory.py | 3 +-- .../options/_soma_tiledb_context.py | 24 +++++++++++-------- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/apis/python/requirements_dev.txt b/apis/python/requirements_dev.txt index f1b06213a8..509e738cbd 100644 --- a/apis/python/requirements_dev.txt +++ b/apis/python/requirements_dev.txt @@ -3,5 +3,5 @@ ruff pytest pytest-cov sparse -typeguard==4.1.5 +typeguard==4.2.1 types-setuptools diff --git a/apis/python/src/tiledbsoma/_collection.py b/apis/python/src/tiledbsoma/_collection.py index ebd5afeec8..2869165e1d 100644 --- a/apis/python/src/tiledbsoma/_collection.py +++ b/apis/python/src/tiledbsoma/_collection.py @@ -185,7 +185,7 @@ def add_new_collection( def add_new_collection( self, key: str, - kind: Optional[Type[AnyTileDBCollection]] = None, + kind: Optional[Type[CollectionBase]] = None, # type: ignore[type-arg] *, uri: Optional[str] = None, platform_config: Optional[options.PlatformConfig] = None, @@ -377,7 +377,6 @@ def add_new_sparse_ndarray(self, key: str, **kwargs: Any) -> SparseNDArray: """ return self._add_new_ndarray(SparseNDArray, key, **kwargs) - @typeguard_ignore def _add_new_element( self, key: str, @@ -444,7 +443,7 @@ def __getitem__(self, key: str) -> CollectionElementType: entry.entry.wrapper_type.open, uri, mode, context, timestamp ) # Since we just opened this object, we own it and should close it. - self._close_stack.enter_context(entry.soma) # type: ignore[arg-type] + self._close_stack.enter_context(entry.soma) return cast(CollectionElementType, entry.soma) def set( diff --git a/apis/python/src/tiledbsoma/_factory.py b/apis/python/src/tiledbsoma/_factory.py index 4ccee18ec3..4369145f0e 100644 --- a/apis/python/src/tiledbsoma/_factory.py +++ b/apis/python/src/tiledbsoma/_factory.py @@ -122,7 +122,7 @@ def open( Experimental. """ context = _validate_soma_tiledb_context(context) - obj: TileDBObject[_Wrapper] = _open_internal( # type: ignore[no-untyped-call,valid-type] + obj: TileDBObject[_Wrapper] = _open_internal( # type: ignore[valid-type] _tdb_handles.open, uri, mode, context, tiledb_timestamp ) try: @@ -143,7 +143,6 @@ def open( raise -@no_type_check def _open_internal( opener: Callable[ [str, options.OpenMode, SOMATileDBContext, Optional[OpenTimestamp]], _Wrapper diff --git a/apis/python/src/tiledbsoma/options/_soma_tiledb_context.py b/apis/python/src/tiledbsoma/options/_soma_tiledb_context.py index 2dcd7f9df3..e4e38349c7 100644 --- a/apis/python/src/tiledbsoma/options/_soma_tiledb_context.py +++ b/apis/python/src/tiledbsoma/options/_soma_tiledb_context.py @@ -7,8 +7,8 @@ import functools import threading import time -from concurrent import futures -from typing import Any, Dict, Mapping, Optional, Union +from concurrent.futures import ThreadPoolExecutor +from typing import Any, Dict, Mapping, Optional, Type, Union, cast import tiledb from somacore import ContextBase @@ -46,8 +46,10 @@ def _maybe_timestamp_ms(input: Optional[OpenTimestamp]) -> Optional[int]: return to_timestamp_ms(input) -_SENTINEL = object() -"""Sentinel object to distinguish default parameters from None.""" +class _SENTINEL: + """Sentinel object to distinguish default parameters from None.""" + + pass class SOMATileDBContext(ContextBase): @@ -68,7 +70,7 @@ def __init__( tiledb_ctx: Optional[tiledb.Ctx] = None, tiledb_config: Optional[Dict[str, Union[str, float]]] = None, timestamp: Optional[OpenTimestamp] = None, - threadpool: Optional[futures.ThreadPoolExecutor] = None, + threadpool: Optional[ThreadPoolExecutor] = None, ) -> None: """Initializes a new SOMATileDBContext. @@ -138,7 +140,7 @@ def __init__( """The TileDB context to use, either provided or lazily constructed.""" self._timestamp_ms = _maybe_timestamp_ms(timestamp) - self.threadpool = threadpool or futures.ThreadPoolExecutor() + self.threadpool = threadpool or ThreadPoolExecutor() """User specified threadpool. If None, we'll instantiate one ourselves.""" self._native_context: Optional[clib.SOMAContext] = None """Lazily construct clib.SOMAContext.""" @@ -219,8 +221,8 @@ def replace( *, tiledb_config: Optional[Dict[str, Any]] = None, tiledb_ctx: Optional[tiledb.Ctx] = None, - timestamp: Optional[OpenTimestamp] = _SENTINEL, # type: ignore[assignment] - threadpool: Optional[futures.ThreadPoolExecutor] = _SENTINEL, # type: ignore[assignment] + timestamp: Union[None, OpenTimestamp, Type[_SENTINEL]] = _SENTINEL, + threadpool: Union[None, ThreadPoolExecutor, Type[_SENTINEL]] = _SENTINEL, ) -> Self: """Create a copy of the context, merging changes. @@ -259,17 +261,19 @@ def replace( new_config = 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} + if timestamp is _SENTINEL: # Keep the existing timestamp if not overridden. timestamp = self._timestamp_ms if threadpool is _SENTINEL: # Keep the existing threadpool if not overridden. threadpool = self.threadpool + return type(self)( tiledb_config=tiledb_config, tiledb_ctx=tiledb_ctx, - timestamp=timestamp, - threadpool=threadpool, + timestamp=cast(Optional[OpenTimestamp], timestamp), + threadpool=cast(Optional[ThreadPoolExecutor], threadpool), ) def _open_timestamp_ms(self, in_timestamp: Optional[OpenTimestamp]) -> int: From 5e750c1ab5840e10b11f66f8c90ac32f7aa36c9d Mon Sep 17 00:00:00 2001 From: Ryan Williams Date: Tue, 26 Mar 2024 15:27:29 -0400 Subject: [PATCH 3/3] cr feedback: `_UNSET`/`_Unset` sentinel value/type --- .../options/_soma_tiledb_context.py | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/apis/python/src/tiledbsoma/options/_soma_tiledb_context.py b/apis/python/src/tiledbsoma/options/_soma_tiledb_context.py index e4e38349c7..c78faf03a3 100644 --- a/apis/python/src/tiledbsoma/options/_soma_tiledb_context.py +++ b/apis/python/src/tiledbsoma/options/_soma_tiledb_context.py @@ -8,7 +8,7 @@ import threading import time from concurrent.futures import ThreadPoolExecutor -from typing import Any, Dict, Mapping, Optional, Type, Union, cast +from typing import Any, Dict, Literal, Mapping, Optional, Union import tiledb from somacore import ContextBase @@ -46,10 +46,8 @@ def _maybe_timestamp_ms(input: Optional[OpenTimestamp]) -> Optional[int]: return to_timestamp_ms(input) -class _SENTINEL: - """Sentinel object to distinguish default parameters from None.""" - - pass +_Unset = Literal["__unset__"] +_UNSET: _Unset = "__unset__" class SOMATileDBContext(ContextBase): @@ -221,8 +219,8 @@ def replace( *, tiledb_config: Optional[Dict[str, Any]] = None, tiledb_ctx: Optional[tiledb.Ctx] = None, - timestamp: Union[None, OpenTimestamp, Type[_SENTINEL]] = _SENTINEL, - threadpool: Union[None, ThreadPoolExecutor, Type[_SENTINEL]] = _SENTINEL, + timestamp: Union[None, OpenTimestamp, _Unset] = _UNSET, + threadpool: Union[None, ThreadPoolExecutor, _Unset] = _UNSET, ) -> Self: """Create a copy of the context, merging changes. @@ -262,18 +260,18 @@ def replace( new_config.update(tiledb_config) tiledb_config = {k: v for (k, v) in new_config.items() if v is not None} - if timestamp is _SENTINEL: + if timestamp == _UNSET: # Keep the existing timestamp if not overridden. timestamp = self._timestamp_ms - if threadpool is _SENTINEL: + if threadpool == _UNSET: # Keep the existing threadpool if not overridden. threadpool = self.threadpool return type(self)( tiledb_config=tiledb_config, tiledb_ctx=tiledb_ctx, - timestamp=cast(Optional[OpenTimestamp], timestamp), - threadpool=cast(Optional[ThreadPoolExecutor], threadpool), + timestamp=timestamp, + threadpool=threadpool, ) def _open_timestamp_ms(self, in_timestamp: Optional[OpenTimestamp]) -> int: