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.

Sign up for GitHub

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

refactor: Avoid implicit reexport in submodules #539

Closed
wants to merge 2 commits into from
Closed

refactor: Avoid implicit reexport in submodules #539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ec204e3
refactor: Avoid implicit reexports in extensions
l0b0 Jul 8, 2021
6ef6656
refactor: Avoid implicit reexports in validation
l0b0 Jul 8, 2021
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
58 changes: 30 additions & 28 deletions pystac/extensions/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,27 +12,33 @@
Union,
)

import pystac
from pystac.asset import Asset
from pystac.errors import STACError
from pystac.summaries import Summaries
from pystac.stac_object import STACObject
from pystac.collection import Collection
from pystac.errors import ExtensionNotImplemented
from pystac.summaries import RangeSummary
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer module only imports. e.g. These are a good balance between import pystac which causes trouble and not having to worry about getting more symbols out of any particular module. So as long as modules names aren't mutated, then the imports do not have to change.

from pystac import asset
from pystac import summaries
from pystac import stac_object
from pystac import collection
from pystac import errors
from pystac import summaries

This also means it's a lot easier to find anything in use from a module within the current file even if the names are not as simple as they often are. e.g. a simple search for asset. will find asset.Asset and asset.SomeHelper

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a nice compromise. My main question would be whether it helps with avoiding circular imports, which are currently plaguing the code base. For example, running isort will immediately break the entire code base, and there seems to be a bunch of duplicate imports (import pystac followed by from pystac import foo) which, if merged, also cause circular import errors.



class SummariesExtension:
"""Base class for extending the properties in :attr:`pystac.Collection.summaries`
"""Base class for extending the properties in :attr:`Collection.summaries`
to include properties defined by a STAC Extension.

This class should generally not be instantiated directly. Instead, create an
extension-specific class that inherits from this class and instantiate that. See
:class:`~pystac.extensions.eo.SummariesEOExtension` for an example."""
:class:`~extensions.eo.SummariesEOExtension` for an example."""

summaries: pystac.Summaries
"""The summaries for the :class:`~pystac.Collection` being extended."""
summaries: Summaries
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from above, this would be summaries: summaries.Summaries. That's a bit wordy, but really obvious where it comes from.

Copy link
Contributor Author

@l0b0 l0b0 Jul 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, fixing the comments is another can of worms I'm not comfortable with tackling until we've nailed the imports themselves. I'm guessing they should resolve within the current set of imports, that is, if I from pystac import summaries the comment should be summaries: summaries.Summaries as you said, rather than summaries: pystac.summaries.Summaries.

"""The summaries for the :class:`~Collection` being extended."""

def __init__(self, collection: pystac.Collection) -> None:
def __init__(self, collection: Collection) -> None:
self.summaries = collection.summaries

def _set_summary(
self,
prop_key: str,
v: Optional[Union[List[Any], pystac.RangeSummary[Any], Dict[str, Any]]],
v: Optional[Union[List[Any], RangeSummary[Any], Dict[str, Any]]],
) -> None:
if v is None:
self.summaries.remove(prop_key)
Expand All @@ -44,12 +50,12 @@ def _set_summary(


class PropertiesExtension(ABC):
"""Abstract base class for extending the properties of an :class:`~pystac.Item`
"""Abstract base class for extending the properties of an :class:`~Item`
to include properties defined by a STAC Extension.

This class should not be instantiated directly. Instead, create an
extension-specific class that inherits from this class and instantiate that. See
:class:`~pystac.extensions.eo.PropertiesEOExtension` for an example.
:class:`~extensions.eo.PropertiesEOExtension` for an example.
"""

properties: Dict[str, Any]
Expand All @@ -62,8 +68,8 @@ class PropertiesExtension(ABC):
additional_read_properties: Optional[Iterable[Dict[str, Any]]] = None
"""Additional read-only properties accessible from the extended object.

These are used when extending an :class:`~pystac.Asset` to give access to the
properties of the owning :class:`~pystac.Item`. If a property exists in both
These are used when extending an :class:`~Asset` to give access to the
properties of the owning :class:`~Item`. If a property exists in both
``additional_read_properties`` and ``properties``, the value in
``additional_read_properties`` will take precedence.
"""
Expand All @@ -88,18 +94,18 @@ def _set_property(
self.properties[prop_name] = v


S = TypeVar("S", bound=pystac.STACObject)
S = TypeVar("S", bound=STACObject)


class ExtensionManagementMixin(Generic[S], ABC):
"""Abstract base class with methods for adding and removing extensions from STAC
Objects. This class is generic over the type of object being extended (e.g.
:class:`~pystac.Item`).
:class:`~Item`).

Concrete extension implementations should inherit from this class and either
provide a concrete type or a bounded type variable.

See :class:`~pystac.extensions.eo.EOExtension` for an example implementation.
See :class:`~extensions.eo.EOExtension` for an example implementation.
"""

@classmethod
Expand All @@ -111,7 +117,7 @@ def get_schema_uri(cls) -> str:
@classmethod
def add_to(cls, obj: S) -> None:
"""Add the schema URI for this extension to the
:attr:`~pystac.STACObject.stac_extensions` list for the given object, if it is
:attr:`~STACObject.stac_extensions` list for the given object, if it is
not already present."""
if obj.stac_extensions is None:
obj.stac_extensions = [cls.get_schema_uri()]
Expand All @@ -121,7 +127,7 @@ def add_to(cls, obj: S) -> None:
@classmethod
def remove_from(cls, obj: S) -> None:
"""Remove the schema URI for this extension from the
:attr:`pystac.STACObject.stac_extensions` list for the given object."""
:attr:`STACObject.stac_extensions` list for the given object."""
if obj.stac_extensions is not None:
obj.stac_extensions = [
uri for uri in obj.stac_extensions if uri != cls.get_schema_uri()
Expand All @@ -130,18 +136,16 @@ def remove_from(cls, obj: S) -> None:
@classmethod
def has_extension(cls, obj: S) -> bool:
"""Check if the given object implements this extension by checking
:attr:`pystac.STACObject.stac_extensions` for this extension's schema URI."""
:attr:`STACObject.stac_extensions` for this extension's schema URI."""
return (
obj.stac_extensions is not None
and cls.get_schema_uri() in obj.stac_extensions
)

@classmethod
def validate_owner_has_extension(
cls, asset: pystac.Asset, add_if_missing: bool
) -> None:
"""Given an :class:`~pystac.Asset`, checks if the asset's owner has this
extension's schema URI in its :attr:`~pystac.STACObject.stac_extensions` list.
def validate_owner_has_extension(cls, asset: Asset, add_if_missing: bool) -> None:
"""Given an :class:`~Asset`, checks if the asset's owner has this
extension's schema URI in its :attr:`~STACObject.stac_extensions` list.
If ``add_if_missing`` is ``True``, the schema URI will be added to the owner.

Raises:
Expand All @@ -150,17 +154,15 @@ def validate_owner_has_extension(
"""
if asset.owner is None:
if add_if_missing:
raise pystac.STACError(
"Can only add schema URIs to Assets with an owner."
)
raise STACError("Can only add schema URIs to Assets with an owner.")
else:
return
return cls.validate_has_extension(cast(S, asset.owner), add_if_missing)

@classmethod
def validate_has_extension(cls, obj: S, add_if_missing: bool) -> None:
"""Given a :class:`~pystac.STACObject`, checks if the object has this
extension's schema URI in its :attr:`~pystac.STACObject.stac_extensions` list.
"""Given a :class:`~STACObject`, checks if the object has this
extension's schema URI in its :attr:`~STACObject.stac_extensions` list.
If ``add_if_missing`` is ``True``, the schema URI will be added to the object.

Args:
Expand All @@ -172,6 +174,6 @@ def validate_has_extension(cls, obj: S, add_if_missing: bool) -> None:
cls.add_to(obj)

if cls.get_schema_uri() not in obj.stac_extensions:
raise pystac.ExtensionNotImplemented(
raise ExtensionNotImplemented(
f"Could not find extension schema URI {cls.get_schema_uri()} in object."
)
44 changes: 24 additions & 20 deletions pystac/extensions/datacube.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,19 @@
from abc import ABC
from typing import Any, Dict, Generic, List, Optional, TypeVar, Union, cast

import pystac
from pystac.asset import Asset
from pystac.stac_object import STACObjectType
from pystac.collection import Collection
from pystac.errors import ExtensionTypeError, RequiredPropertyMissing
from pystac.item import Item
from pystac.extensions.base import (
ExtensionManagementMixin,
PropertiesExtension,
)
from pystac.extensions.hooks import ExtensionHooks
from pystac.utils import get_required, map_opt

T = TypeVar("T", pystac.Collection, pystac.Item, pystac.Asset)
T = TypeVar("T", Collection, Item, Asset)

SCHEMA_URI = "https://stac-extensions.github.io/datacube/v1.0.0/schema.json"

Expand Down Expand Up @@ -65,11 +69,11 @@ def to_dict(self) -> Dict[str, Any]:
def from_dict(d: Dict[str, Any]) -> "Dimension":
dim_type = d.get(DIM_TYPE_PROP)
if dim_type is None:
raise pystac.RequiredPropertyMissing("cube_dimension", DIM_TYPE_PROP)
raise RequiredPropertyMissing("cube_dimension", DIM_TYPE_PROP)
if dim_type == "spatial":
axis = d.get(DIM_AXIS_PROP)
if axis is None:
raise pystac.RequiredPropertyMissing("cube_dimension", DIM_AXIS_PROP)
raise RequiredPropertyMissing("cube_dimension", DIM_AXIS_PROP)
if axis == "z":
return VerticalSpatialDimension(d)
else:
Expand Down Expand Up @@ -313,7 +317,7 @@ def reference_system(self, v: Optional[Union[str, float, Dict[str, Any]]]) -> No
class DatacubeExtension(
Generic[T],
PropertiesExtension,
ExtensionManagementMixin[Union[pystac.Collection, pystac.Item]],
ExtensionManagementMixin[Union[Collection, Item]],
):
def apply(self, dimensions: Dict[str, Dimension]) -> None:
self.dimensions = dimensions
Expand All @@ -339,54 +343,54 @@ def get_schema_uri(cls) -> str:

@classmethod
def ext(cls, obj: T, add_if_missing: bool = False) -> "DatacubeExtension[T]":
if isinstance(obj, pystac.Collection):
if isinstance(obj, Collection):
cls.validate_has_extension(obj, add_if_missing)
return cast(DatacubeExtension[T], CollectionDatacubeExtension(obj))
if isinstance(obj, pystac.Item):
if isinstance(obj, Item):
cls.validate_has_extension(obj, add_if_missing)
return cast(DatacubeExtension[T], ItemDatacubeExtension(obj))
elif isinstance(obj, pystac.Asset):
elif isinstance(obj, Asset):
cls.validate_owner_has_extension(obj, add_if_missing)
return cast(DatacubeExtension[T], AssetDatacubeExtension(obj))
else:
raise pystac.ExtensionTypeError(
raise ExtensionTypeError(
f"Datacube extension does not apply to type '{type(obj).__name__}'"
)


class CollectionDatacubeExtension(DatacubeExtension[pystac.Collection]):
collection: pystac.Collection
class CollectionDatacubeExtension(DatacubeExtension[Collection]):
collection: Collection
properties: Dict[str, Any]

def __init__(self, collection: pystac.Collection):
def __init__(self, collection: Collection):
self.collection = collection
self.properties = collection.extra_fields

def __repr__(self) -> str:
return "<CollectionDatacubeExtension Item id={}>".format(self.collection.id)


class ItemDatacubeExtension(DatacubeExtension[pystac.Item]):
item: pystac.Item
class ItemDatacubeExtension(DatacubeExtension[Item]):
item: Item
properties: Dict[str, Any]

def __init__(self, item: pystac.Item):
def __init__(self, item: Item):
self.item = item
self.properties = item.properties

def __repr__(self) -> str:
return "<ItemDatacubeExtension Item id={}>".format(self.item.id)


class AssetDatacubeExtension(DatacubeExtension[pystac.Asset]):
class AssetDatacubeExtension(DatacubeExtension[Asset]):
asset_href: str
properties: Dict[str, Any]
additional_read_properties: Optional[List[Dict[str, Any]]]

def __init__(self, asset: pystac.Asset):
def __init__(self, asset: Asset):
self.asset_href = asset.href
self.properties = asset.extra_fields
if asset.owner and isinstance(asset.owner, pystac.Item):
if asset.owner and isinstance(asset.owner, Item):
self.additional_read_properties = [asset.owner.properties]
else:
self.additional_read_properties = None
Expand All @@ -399,8 +403,8 @@ class DatacubeExtensionHooks(ExtensionHooks):
schema_uri: str = SCHEMA_URI
prev_extension_ids = {"datacube"}
stac_object_types = {
pystac.STACObjectType.COLLECTION,
pystac.STACObjectType.ITEM,
STACObjectType.COLLECTION,
STACObjectType.ITEM,
}


Expand Down
Loading