From fadf49089e53ec3f25c44e2e032afcdf38d263da Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Fri, 25 Jun 2021 11:26:19 -0500 Subject: [PATCH 1/3] REF: Use descriptors for properties --- pystac/extensions/datacube.py | 308 ++++++++++-------------------- tests/extensions/test_datacube.py | 40 +++- 2 files changed, 135 insertions(+), 213 deletions(-) diff --git a/pystac/extensions/datacube.py b/pystac/extensions/datacube.py index e43b8de4c..2dbfaba95 100644 --- a/pystac/extensions/datacube.py +++ b/pystac/extensions/datacube.py @@ -4,7 +4,18 @@ """ from abc import ABC -from typing import Any, Dict, Generic, List, Optional, Set, TypeVar, Union, cast +from typing import ( + Any, + Dict, + Generic, + List, + Optional, + Protocol, + Set, + TypeVar, + Union, + cast, +) import pystac from pystac.extensions.base import ( @@ -30,33 +41,65 @@ DIM_REF_SYS_PROP = "reference_system" DIM_UNIT_PROP = "unit" +U = TypeVar("U") -class Dimension(ABC): - properties: Dict[str, Any] - - def __init__(self, properties: Dict[str, Any]) -> None: - self.properties = properties +class HasProperties(Protocol[U]): @property - def dim_type(self) -> str: - return get_required( - self.properties.get(DIM_TYPE_PROP), "cube:dimension", DIM_TYPE_PROP - ) + def properties(self) -> Dict[str, U]: + """A dictionary with the properties for the object.""" + + +class _Property(Generic[U]): + """ + A descriptor managing the properties of an object. + + Args: + key : The key within the managed object's ``properties`` dictionary to use. + obj : The key this object is contained with (e.g. ``"cube:variables"``) + required : Whether the property is required. + clear_if_none : Whether setting a value of `None` clears the value from + the ``properties`` dictionary. + """ + + def __init__( + self, + key: str, + obj: Union[str, Any] = None, + required: bool = False, + clear_if_none: Optional[bool] = None, + ): + self.key = key + self.obj = obj + self.required = required + if clear_if_none is None: + clear_if_none = not required + self.clear_if_none = clear_if_none + + def __get__(self, instance: HasProperties[U], owner: Any = None) -> Optional[U]: + properties: Dict[str, U] = getattr(instance, "properties") + if self.required: + return get_required(properties.get(self.key), self.obj, self.key) + else: + return properties.get(self.key) - @dim_type.setter - def dim_type(self, v: str) -> None: - self.properties[DIM_TYPE_PROP] = v + def __set__(self, obj: HasProperties[U], value: U) -> None: + properties = getattr(obj, "properties") + if value is None and self.clear_if_none: + properties.pop(self.key, None) + else: + properties[self.key] = value - @property - def description(self) -> Optional[str]: - return self.properties.get(DIM_DESC_PROP) - @description.setter - def description(self, v: Optional[str]) -> None: - if v is None: - self.properties.pop(DIM_DESC_PROP, None) - else: - self.properties[DIM_DESC_PROP] = v +class Dimension(ABC): + properties: Dict[str, Any] + dim_type: _Property[str] = _Property( + DIM_TYPE_PROP, "cube:dimensions", required=True + ) + description: _Property[Optional[str]] = _Property(DIM_DESC_PROP) + + def __init__(self, properties: Dict[str, Any]) -> None: + self.properties = properties def to_dict(self) -> Dict[str, Any]: return self.properties @@ -85,44 +128,15 @@ def from_dict(d: Dict[str, Any]) -> "Dimension": class HorizontalSpatialDimension(Dimension): - @property - def axis(self) -> str: - return get_required( - self.properties.get(DIM_AXIS_PROP), "cube:dimension", DIM_AXIS_PROP - ) - - @axis.setter - def axis(self, v: str) -> None: - self.properties[DIM_TYPE_PROP] = v - - @property - def extent(self) -> List[float]: - return get_required( - self.properties.get(DIM_EXTENT_PROP), "cube:dimension", DIM_EXTENT_PROP - ) - - @extent.setter - def extent(self, v: List[float]) -> None: - self.properties[DIM_EXTENT_PROP] = v - - @property - def values(self) -> Optional[List[float]]: - return self.properties.get(DIM_VALUES_PROP) - - @values.setter - def values(self, v: Optional[List[float]]) -> None: - if v is None: - self.properties.pop(DIM_VALUES_PROP, None) - else: - self.properties[DIM_VALUES_PROP] = v - - @property - def step(self) -> Optional[float]: - return self.properties.get(DIM_STEP_PROP) - - @step.setter - def step(self, v: Optional[float]) -> None: - self.properties[DIM_STEP_PROP] = v + axis: _Property[str] = _Property(DIM_AXIS_PROP, "cube:dimensions", required=True) + extent: _Property[List[float]] = _Property( + DIM_EXTENT_PROP, "cube:dimensions", required=True + ) + values: _Property[Optional[List[float]]] = _Property(DIM_VALUES_PROP) + step: _Property[Optional[float]] = _Property(DIM_STEP_PROP, clear_if_none=False) + reference_system: _Property[ + Optional[Union[str, float, Dict[str, Any]]] + ] = _Property(DIM_REF_SYS_PROP) def clear_step(self) -> None: """Setting step to None sets it to the null value, @@ -130,58 +144,18 @@ def clear_step(self) -> None: to remove it from the properties.""" self.properties.pop(DIM_STEP_PROP, None) - @property - def reference_system(self) -> Optional[Union[str, float, Dict[str, Any]]]: - return self.properties.get(DIM_REF_SYS_PROP) - - @reference_system.setter - def reference_system(self, v: Optional[Union[str, float, Dict[str, Any]]]) -> None: - if v is None: - self.properties.pop(DIM_REF_SYS_PROP, None) - else: - self.properties[DIM_REF_SYS_PROP] = v - class VerticalSpatialDimension(Dimension): - @property - def axis(self) -> str: - return get_required( - self.properties.get(DIM_AXIS_PROP), "cube:dimension", DIM_AXIS_PROP - ) - - @axis.setter - def axis(self, v: str) -> None: - self.properties[DIM_TYPE_PROP] = v - - @property - def extent(self) -> Optional[List[Optional[float]]]: - return self.properties.get(DIM_EXTENT_PROP) - - @extent.setter - def extent(self, v: Optional[List[Optional[float]]]) -> None: - if v is None: - self.properties.pop(DIM_EXTENT_PROP, None) - else: - self.properties[DIM_EXTENT_PROP] = v - - @property - def values(self) -> Optional[Union[List[float], List[str]]]: - return self.properties.get(DIM_VALUES_PROP) - - @values.setter - def values(self, v: Optional[Union[List[float], List[str]]]) -> None: - if v is None: - self.properties.pop(DIM_VALUES_PROP, None) - else: - self.properties[DIM_VALUES_PROP] = v - - @property - def step(self) -> Optional[float]: - return self.properties.get(DIM_STEP_PROP) - - @step.setter - def step(self, v: Optional[float]) -> None: - self.properties[DIM_STEP_PROP] = v + axis: _Property[str] = _Property(DIM_AXIS_PROP, required=True) + extent: _Property[Optional[List[Optional[float]]]] = _Property(DIM_EXTENT_PROP) + values: _Property[Optional[Union[List[float], List[str]]]] = _Property( + DIM_VALUES_PROP + ) + step: _Property[Optional[float]] = _Property(DIM_STEP_PROP, clear_if_none=False) + unit: _Property[Optional[str]] = _Property(DIM_UNIT_PROP) + reference_system: _Property[ + Optional[Union[str, float, Dict[str, Any]]] + ] = _Property(DIM_REF_SYS_PROP) def clear_step(self) -> None: """Setting step to None sets it to the null value, @@ -189,59 +163,11 @@ def clear_step(self) -> None: to remove it from the properties.""" self.properties.pop(DIM_STEP_PROP, None) - @property - def unit(self) -> Optional[str]: - return self.properties.get(DIM_UNIT_PROP) - - @unit.setter - def unit(self, v: Optional[str]) -> None: - if v is None: - self.properties.pop(DIM_UNIT_PROP, None) - else: - self.properties[DIM_UNIT_PROP] = v - - @property - def reference_system(self) -> Optional[Union[str, float, Dict[str, Any]]]: - return self.properties.get(DIM_REF_SYS_PROP) - - @reference_system.setter - def reference_system(self, v: Optional[Union[str, float, Dict[str, Any]]]) -> None: - if v is None: - self.properties.pop(DIM_REF_SYS_PROP, None) - else: - self.properties[DIM_REF_SYS_PROP] = v - class TemporalDimension(Dimension): - @property - def extent(self) -> Optional[List[Optional[str]]]: - return self.properties.get(DIM_EXTENT_PROP) - - @extent.setter - def extent(self, v: Optional[List[Optional[str]]]) -> None: - if v is None: - self.properties.pop(DIM_EXTENT_PROP, None) - else: - self.properties[DIM_EXTENT_PROP] = v - - @property - def values(self) -> Optional[List[str]]: - return self.properties.get(DIM_VALUES_PROP) - - @values.setter - def values(self, v: Optional[List[str]]) -> None: - if v is None: - self.properties.pop(DIM_VALUES_PROP, None) - else: - self.properties[DIM_VALUES_PROP] = v - - @property - def step(self) -> Optional[str]: - return self.properties.get(DIM_STEP_PROP) - - @step.setter - def step(self, v: Optional[str]) -> None: - self.properties[DIM_STEP_PROP] = v + extent: _Property[Optional[List[Optional[str]]]] = _Property(DIM_EXTENT_PROP) + values: _Property[Optional[List[str]]] = _Property(DIM_VALUES_PROP) + step: _Property[Optional[str]] = _Property(DIM_STEP_PROP, clear_if_none=False) def clear_step(self) -> None: """Setting step to None sets it to the null value, @@ -251,35 +177,15 @@ def clear_step(self) -> None: class AdditionalDimension(Dimension): - @property - def extent(self) -> Optional[List[Optional[float]]]: - return self.properties.get(DIM_EXTENT_PROP) - - @extent.setter - def extent(self, v: Optional[List[Optional[float]]]) -> None: - if v is None: - self.properties.pop(DIM_EXTENT_PROP, None) - else: - self.properties[DIM_EXTENT_PROP] = v - - @property - def values(self) -> Optional[Union[List[str], List[float]]]: - return self.properties.get(DIM_VALUES_PROP) - - @values.setter - def values(self, v: Optional[Union[List[str], List[float]]]) -> None: - if v is None: - self.properties.pop(DIM_VALUES_PROP, None) - else: - self.properties[DIM_VALUES_PROP] = v - - @property - def step(self) -> Optional[float]: - return self.properties.get(DIM_STEP_PROP) - - @step.setter - def step(self, v: Optional[float]) -> None: - self.properties[DIM_STEP_PROP] = v + extent: _Property[Optional[List[Optional[float]]]] = _Property(DIM_EXTENT_PROP) + values: _Property[Optional[Union[List[str], List[float]]]] = _Property( + DIM_VALUES_PROP + ) + step: _Property[Optional[float]] = _Property(DIM_STEP_PROP, clear_if_none=False) + unit: _Property[Optional[str]] = _Property(DIM_UNIT_PROP) + reference_system: _Property[ + Optional[Union[str, float, Dict[str, Any]]] + ] = _Property(DIM_REF_SYS_PROP) def clear_step(self) -> None: """Setting step to None sets it to the null value, @@ -287,28 +193,6 @@ def clear_step(self) -> None: to remove it from the properties.""" self.properties.pop(DIM_STEP_PROP, None) - @property - def unit(self) -> Optional[str]: - return self.properties.get(DIM_UNIT_PROP) - - @unit.setter - def unit(self, v: Optional[str]) -> None: - if v is None: - self.properties.pop(DIM_UNIT_PROP, None) - else: - self.properties[DIM_UNIT_PROP] = v - - @property - def reference_system(self) -> Optional[Union[str, float, Dict[str, Any]]]: - return self.properties.get(DIM_REF_SYS_PROP) - - @reference_system.setter - def reference_system(self, v: Optional[Union[str, float, Dict[str, Any]]]) -> None: - if v is None: - self.properties.pop(DIM_REF_SYS_PROP, None) - else: - self.properties[DIM_REF_SYS_PROP] = v - class DatacubeExtension( Generic[T], diff --git a/tests/extensions/test_datacube.py b/tests/extensions/test_datacube.py index 251bd7f83..47d9f1233 100644 --- a/tests/extensions/test_datacube.py +++ b/tests/extensions/test_datacube.py @@ -1,6 +1,7 @@ +from typing import cast import unittest import pystac -from pystac.extensions.datacube import DatacubeExtension +from pystac.extensions.datacube import AdditionalDimension, DatacubeExtension from tests.utils import TestCases @@ -14,6 +15,31 @@ def test_validate_datacube(self) -> None: item = pystac.Item.from_file(self.example_uri) item.validate() + def test_get_set(self) -> None: + item = pystac.Item.from_file(self.example_uri) + item.validate() + ext = DatacubeExtension.ext(item) + dim = ext.dimensions["x"] + + assert dim.dim_type == "spatial" + + spectral = ext.dimensions["spectral"] + spectral = cast(AdditionalDimension, spectral) + + assert spectral.values == ["red", "green", "blue"] + + del dim.properties["type"] + + with self.assertRaises(pystac.errors.RequiredPropertyMissing): + dim.dim_type + + dim.dim_type = "spatial" + dim.description = None + + assert "description" not in dim.properties + dim.dim_type = None # type: ignore + assert "type" in dim.properties + def test_extension_not_implemented(self) -> None: # Should raise exception if Item does not include extension URI item = pystac.Item.from_file(self.example_uri) @@ -50,3 +76,15 @@ def test_asset_ext_add_to(self) -> None: _ = DatacubeExtension.ext(asset, add_if_missing=True) self.assertIn(DatacubeExtension.get_schema_uri(), item.stac_extensions) + + def test_clear_step(self) -> None: + item = pystac.Item.from_file(self.example_uri) + ext = DatacubeExtension.ext(item) + dim = ext.dimensions["pressure_levels"] + assert isinstance(dim, AdditionalDimension) + + assert dim.step == 100 + dim.step = None + assert "step" in dim.properties + dim.clear_step() + assert "step" not in dim.properties From 476b20fedbbd812c58993975cb16287b58421072 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Tue, 29 Jun 2021 10:43:35 -0500 Subject: [PATCH 2/3] Fixups * unittest asserts * fixed dim type --- tests/extensions/test_datacube.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/tests/extensions/test_datacube.py b/tests/extensions/test_datacube.py index 47d9f1233..8e2de2b78 100644 --- a/tests/extensions/test_datacube.py +++ b/tests/extensions/test_datacube.py @@ -1,7 +1,11 @@ from typing import cast import unittest import pystac -from pystac.extensions.datacube import AdditionalDimension, DatacubeExtension +from pystac.extensions.datacube import ( + AdditionalDimension, + DatacubeExtension, + VerticalSpatialDimension, +) from tests.utils import TestCases @@ -21,12 +25,12 @@ def test_get_set(self) -> None: ext = DatacubeExtension.ext(item) dim = ext.dimensions["x"] - assert dim.dim_type == "spatial" + self.assertEqual(dim.dim_type, "spatial") spectral = ext.dimensions["spectral"] spectral = cast(AdditionalDimension, spectral) - assert spectral.values == ["red", "green", "blue"] + self.assertEqual(spectral.values, ["red", "green", "blue"]) del dim.properties["type"] @@ -36,9 +40,9 @@ def test_get_set(self) -> None: dim.dim_type = "spatial" dim.description = None - assert "description" not in dim.properties + self.assertNotIn("description", dim.properties) dim.dim_type = None # type: ignore - assert "type" in dim.properties + self.assertIn("type", dim.properties) def test_extension_not_implemented(self) -> None: # Should raise exception if Item does not include extension URI @@ -81,10 +85,11 @@ def test_clear_step(self) -> None: item = pystac.Item.from_file(self.example_uri) ext = DatacubeExtension.ext(item) dim = ext.dimensions["pressure_levels"] - assert isinstance(dim, AdditionalDimension) + self.assertIsInstance(dim, VerticalSpatialDimension) - assert dim.step == 100 + dim = cast(VerticalSpatialDimension, dim) + self.assertEqual(dim.step, 100) dim.step = None - assert "step" in dim.properties + self.assertIn("step", dim.properties) dim.clear_step() - assert "step" not in dim.properties + self.assertNotIn("step", dim.properties) From 89bfa488c8149183a718f77469f1dd28c68db8b2 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Tue, 29 Jun 2021 13:16:26 -0500 Subject: [PATCH 3/3] py37 compat --- pystac/extensions/datacube.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pystac/extensions/datacube.py b/pystac/extensions/datacube.py index 2dbfaba95..df3451844 100644 --- a/pystac/extensions/datacube.py +++ b/pystac/extensions/datacube.py @@ -2,15 +2,14 @@ https://github.com/stac-extensions/datacube """ - from abc import ABC +import sys from typing import ( Any, Dict, Generic, List, Optional, - Protocol, Set, TypeVar, Union, @@ -25,6 +24,11 @@ from pystac.extensions.hooks import ExtensionHooks from pystac.utils import get_required, map_opt +if sys.version_info >= (3, 8): + from typing import Protocol +else: + from typing_extensions import Protocol + T = TypeVar("T", pystac.Collection, pystac.Item, pystac.Asset) SCHEMA_URI = "https://stac-extensions.github.io/datacube/v1.0.0/schema.json"