Skip to content

Commit

Permalink
Don't use TypeVar when defining coordinate types.
Browse files Browse the repository at this point in the history
My previous change which used TypeVar for coordinate types [1] was
actually a Bad Idea.  While it was better in that it prohibited
heterogeneous collections and slices, it was worse in that made the
SparseDFCoord type (and derived types) into generic types, which
we do not want.

The *right* way to avoid repeating all the possible types is to make
a union of the possible *containers*, and then specify *that* with all
the possible contents that may go into those containers.

[1]: #141
  • Loading branch information
thetorpedodog committed Mar 3, 2023
1 parent f923012 commit d514e4b
Showing 1 changed file with 14 additions and 26 deletions.
40 changes: 14 additions & 26 deletions python-spec/src/somacore/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,20 +110,7 @@ class ResultOrder(enum.Enum):
ResultOrderStr = Union[ResultOrder, Literal["auto", "row-major", "column-major"]]
"""A ResultOrder, or the str representing it."""

# Coordinate types are specified as TypeVars instead of Unions to ensure that
# sequences and slices are homogeneous:
#
# BadCoord = Union[int, str]
# BadCoords = Union[BadCoord, Sequence[BadCoord]]
# # ["this", 1] is bad, but is a valid as a BadCoords value
#
# GoodCoord = TypeVar("GoodCoord", int, str)
# GoodCoords = Union[GoodCoord, Sequence[GoodCoord]]
# # ["this", 1] is bad, and is *not* valid as a GoodCoords value
# # ["this", "that"] is a valid as a GoodCoords value
# # [1, 2] is valid as a GoodCoords value

DenseCoord = TypeVar("DenseCoord", None, int, types.Slice[int])
DenseCoord = Union[None, int, types.Slice[int]]
"""A single coordinate range for reading dense data.
``None`` indicates the entire domain of a dimension; values of this type are
Expand All @@ -134,18 +121,21 @@ class ResultOrder(enum.Enum):
DenseNDCoords = Sequence[DenseCoord]
"""A sequence of ranges to read dense data."""

SparseCoord = TypeVar(
"SparseCoord", bytes, float, int, slice, str, np.datetime64, pa.TimestampType
)
"""Types that can be put into a single sparse coordinate."""
_T = TypeVar("_T")
ValSliceOrSequence = Union[_T, types.Slice[_T], types.Sequence[_T]]
"""A value of a type, a Slice of that type, or a Sequence of that type."""

# NOTE: Keep this in sync with the types accepted in `_canonicalize_coord`
# in ./query/axis.py.
SparseDFCoord = Union[
DenseCoord,
SparseCoord,
Sequence[SparseCoord],
types.Slice[SparseCoord],
None,
ValSliceOrSequence[bytes],
ValSliceOrSequence[float],
ValSliceOrSequence[int],
ValSliceOrSequence[slice],
ValSliceOrSequence[str],
ValSliceOrSequence[np.datetime64],
ValSliceOrSequence[pa.TimestampType],
pa.Array,
pa.ChunkedArray,
npt.NDArray[np.integer],
Expand All @@ -156,13 +146,11 @@ class ResultOrder(enum.Enum):
"""A sequence of coordinate ranges for reading dense dataframes."""

SparseNDCoord = Union[
DenseCoord,
Sequence[int],
None,
ValSliceOrSequence[int],
npt.NDArray[np.integer],
pa.IntegerArray,
]
"""A single coordinate range for one dimension of a sparse nd-array."""


SparseNDCoords = Sequence[SparseNDCoord]
"""A sequence of coordinate ranges for reading sparse ndarrays."""

0 comments on commit d514e4b

Please sign in to comment.