Skip to content

Commit

Permalink
Simplify handling of nullability in ColumnSpecs.
Browse files Browse the repository at this point in the history
  • Loading branch information
TallJimbo committed Dec 21, 2023
1 parent 4f3a7a9 commit 61812a9
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 89 deletions.
60 changes: 3 additions & 57 deletions python/lsst/daf/butler/column_spec.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@

__all__ = (
"ColumnSpec",
"not_nullable",
"default_nullable",
"IntColumnSpec",
"StringColumnSpec",
"HashColumnSpec",
Expand Down Expand Up @@ -62,16 +60,9 @@ class _BaseColumnSpec(_BaseModelCompat, ABC):

type: str

nullable: bool | None = pydantic.Field(
default=None,
description=textwrap.dedent(
"""\
Whether the column may be ``NULL``.
The default (represented by `None`) is context-dependent; some
fields are ``NOT NULL``, while oethers default to nullable.
"""
),
nullable: bool = pydantic.Field(
default=True,
description="Whether the column may be ``NULL``.",
)

def to_sql_spec(self, **kwargs: Any) -> ddl.FieldSpec:
Expand Down Expand Up @@ -133,46 +124,6 @@ def __str__(self) -> str:
return "\n".join(self.display())

Check warning on line 124 in python/lsst/daf/butler/column_spec.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/column_spec.py#L124

Added line #L124 was not covered by tests


def not_nullable(c: _BaseColumnSpec) -> _BaseColumnSpec:
"""Pydantic validator for `ColumnSpec` that requires
`ColumnSpec.nullable` to be `False`, and replaces `None` with `False`.
Parameters
----------
c : `ColumnSpec`
Original column specification.
Returns
-------
c : `ColumnSpec`
The object passed in, modified in-place.
"""
if c.nullable is True:
raise ValueError("Key columns may not be nullable.")
c.nullable = False
return c


def default_nullable(c: _BaseColumnSpec) -> _BaseColumnSpec:
"""Pydantic validator for `ColumnSpec` that allows
`ColumnSpec.nullable` to be `True` or `False` and replaces `None` with
`True`.
Parameters
----------
c : `ColumnSpec`
Original column specification.
Returns
-------
c : `ColumnSpec`
The object passed in, modified in-place.
"""
if c.nullable is None:
c.nullable = True
return c


@final
class IntColumnSpec(_BaseColumnSpec):
"""Description of an integer column."""
Expand All @@ -183,7 +134,6 @@ class IntColumnSpec(_BaseColumnSpec):

def to_arrow(self) -> arrow_utils.ToArrow:
# Docstring inherited.
assert self.nullable is not None, "nullable=None should be resolved by validators"
return arrow_utils.ToArrow.for_primitive(self.name, pa.uint64(), nullable=self.nullable)


Expand All @@ -204,7 +154,6 @@ def to_sql_spec(self, **kwargs: Any) -> ddl.FieldSpec:

def to_arrow(self) -> arrow_utils.ToArrow:
# Docstring inherited.
assert self.nullable is not None, "nullable=None should be resolved by validators"
return arrow_utils.ToArrow.for_primitive(self.name, pa.string(), nullable=self.nullable)


Expand All @@ -225,7 +174,6 @@ def to_sql_spec(self, **kwargs: Any) -> ddl.FieldSpec:

def to_arrow(self) -> arrow_utils.ToArrow:
# Docstring inherited.
assert self.nullable is not None, "nullable=None should be resolved by validators"
return arrow_utils.ToArrow.for_primitive(
self.name,
# The size for Arrow binary columns is a fixed size, not a maximum
Expand Down Expand Up @@ -259,7 +207,6 @@ class BoolColumnSpec(_BaseColumnSpec):

def to_arrow(self) -> arrow_utils.ToArrow:
# Docstring inherited.
assert self.nullable is not None, "nullable=None should be resolved by validators"
return arrow_utils.ToArrow.for_primitive(self.name, pa.bool_(), nullable=self.nullable)

Check warning on line 210 in python/lsst/daf/butler/column_spec.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/column_spec.py#L210

Added line #L210 was not covered by tests


Expand Down Expand Up @@ -294,7 +241,6 @@ class TimespanColumnSpec(_BaseColumnSpec):

def to_arrow(self) -> arrow_utils.ToArrow:
# Docstring inherited.
assert self.nullable is not None, "nullable=None should be resolved by validators"
return arrow_utils.ToArrow.for_timespan(self.name, nullable=self.nullable)


Expand Down
28 changes: 7 additions & 21 deletions python/lsst/daf/butler/dimensions/_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,14 @@

import warnings
from collections.abc import Iterator, Mapping, Sequence
from typing import Annotated, Any, TypeAlias
from typing import Any

import pydantic
from lsst.resources import ResourcePath, ResourcePathExpression

from .._compat import PYDANTIC_V2
from .._config import Config, ConfigSubset
from .._topology import TopologicalSpace
from ..column_spec import default_nullable, not_nullable
from ._database import (
DatabaseDimensionElementConstructionVisitor,
DatabaseTopologicalFamilyConstructionVisitor,
Expand All @@ -54,21 +53,6 @@
# have a version.
_DEFAULT_NAMESPACE = "daf_butler"

if PYDANTIC_V2:
AnnotatedKeyColumnSpec: TypeAlias = Annotated[
KeyColumnSpec, pydantic.Field(discriminator="type"), pydantic.AfterValidator(not_nullable)
]
AnnotatedMetadataColumnSpec: TypeAlias = Annotated[
MetadataColumnSpec, pydantic.Field(discriminator="type"), pydantic.AfterValidator(default_nullable)
]
else:

class _KeyColumnModel(pydantic.BaseModel):
spec: KeyColumnSpec = pydantic.Field(discriminator="type")

class _MetadataColumnModel(pydantic.BaseModel):
spec: MetadataColumnSpec = pydantic.Field(discriminator="type")


class DimensionConfig(ConfigSubset):
"""Configuration that defines a `DimensionUniverse`.
Expand Down Expand Up @@ -214,24 +198,26 @@ def _extractElementVisitors(self) -> Iterator[DimensionConstructionVisitor]:
# MyPy is confused by the typing.Annotated usage and/or how
# Pydantic annotated TypeAdapter.
key_adapter: pydantic.TypeAdapter[KeyColumnSpec] = pydantic.TypeAdapter( # type: ignore
AnnotatedKeyColumnSpec # type: ignore
KeyColumnSpec # type: ignore
)
validate_key = key_adapter.validate_python
metadata_adapter: pydantic.TypeAdapter[MetadataColumnSpec] = pydantic.TypeAdapter( # type: ignore
AnnotatedMetadataColumnSpec # type: ignore
MetadataColumnSpec # type: ignore
)
validate_metadata = metadata_adapter.validate_python
else:

def validate_key(value: Any) -> KeyColumnSpec: # type: ignore[misc]
return not_nullable(_KeyColumnModel(spec=value).spec) # type: ignore[return-value]
return pydantic.parse_obj_as(KeyColumnSpec, value) # type: ignore

Check warning on line 211 in python/lsst/daf/butler/dimensions/_config.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/dimensions/_config.py#L210-L211

Added lines #L210 - L211 were not covered by tests

def validate_metadata(value: Any) -> MetadataColumnSpec: # type: ignore[misc]
return default_nullable(_MetadataColumnModel(spec=value).spec) # type: ignore[return-value]
return pydantic.parse_obj_as(MetadataColumnSpec, value) # type: ignore

Check warning on line 214 in python/lsst/daf/butler/dimensions/_config.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/dimensions/_config.py#L213-L214

Added lines #L213 - L214 were not covered by tests

for name, subconfig in self["elements"].items():
metadata_columns = [validate_metadata(c) for c in subconfig.get("metadata", ())]
unique_keys = [validate_key(c) for c in subconfig.get("keys", ())]
for unique_key in unique_keys:
unique_key.nullable = False
if subconfig.get("governor", False):
unsupported = {"required", "implied", "viewOf", "alwaysJoin"}
if not unsupported.isdisjoint(subconfig.keys()):
Expand Down
29 changes: 18 additions & 11 deletions python/lsst/daf/butler/dimensions/_elements.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,9 @@
)

from abc import abstractmethod
from typing import TYPE_CHECKING, Any, ClassVar, TypeAlias, Union, cast
from typing import TYPE_CHECKING, Annotated, Any, ClassVar, TypeAlias, Union, cast

import pydantic
from lsst.utils.classes import cached_getter

from .. import arrow_utils, column_spec, ddl
Expand All @@ -52,18 +53,24 @@
from ._schema import DimensionRecordSchema
from ._universe import DimensionUniverse

KeyColumnSpec: TypeAlias = Union[
column_spec.IntColumnSpec,
column_spec.StringColumnSpec,
column_spec.HashColumnSpec,
KeyColumnSpec: TypeAlias = Annotated[
Union[
column_spec.IntColumnSpec,
column_spec.StringColumnSpec,
column_spec.HashColumnSpec,
],
pydantic.Field(discriminator="type"),
]

MetadataColumnSpec: TypeAlias = Union[
column_spec.IntColumnSpec,
column_spec.StringColumnSpec,
column_spec.FloatColumnSpec,
column_spec.HashColumnSpec,
column_spec.BoolColumnSpec,
MetadataColumnSpec: TypeAlias = Annotated[
Union[
column_spec.IntColumnSpec,
column_spec.StringColumnSpec,
column_spec.FloatColumnSpec,
column_spec.HashColumnSpec,
column_spec.BoolColumnSpec,
],
pydantic.Field(discriminator="type"),
]


Expand Down

0 comments on commit 61812a9

Please sign in to comment.