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.

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

Fix mypy support #337

Merged
merged 7 commits into from
May 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
import os
import sys
import subprocess
from typing import Any, Dict

sys.path.insert(0, os.path.abspath('.'))
sys.path.insert(0, os.path.abspath('../'))
from pystac.version import __version__
Expand Down Expand Up @@ -134,7 +136,7 @@

# -- Options for LaTeX output ------------------------------------------------

latex_elements = {
latex_elements: Dict[str, Any] = {
# The paper size ('letterpaper' or 'a4paper').
#
# 'papersize': 'letterpaper',
Expand Down
3 changes: 2 additions & 1 deletion mypy.ini
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
[mypy]
disallow_untyped_defs = True
ignore_missing_imports = True
disallow_untyped_defs = True
show_error_codes = True
8 changes: 4 additions & 4 deletions pystac/collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -431,9 +431,9 @@ def to_dict(self) -> Dict[str, Any]:
return {"minimum": self.minimum, "maximum": self.maximum}

@classmethod
def from_dict(cls, d: Dict[str, Any], typ: Type[T] = Any) -> "RangeSummary[T]":
minimum: Optional[T] = get_required(d.get("minimum"), "RangeSummary", "minimum")
maximum: Optional[T] = get_required(d.get("maximum"), "RangeSummary", "maximum")
def from_dict(cls, d: Dict[str, Any]) -> "RangeSummary[T]":
minimum: T = get_required(d.get("minimum"), "RangeSummary", "minimum")
maximum: T = get_required(d.get("maximum"), "RangeSummary", "maximum")
return cls(minimum=minimum, maximum=maximum)


Expand Down Expand Up @@ -483,7 +483,7 @@ def remove(self, prop_key: str) -> None:
self.schemas.pop(prop_key, None)
self.other.pop(prop_key, None)

def is_empty(self):
def is_empty(self) -> bool:
return not (
any(self.lists) or any(self.ranges) or any(self.schemas) or any(self.other)
)
Expand Down
14 changes: 8 additions & 6 deletions pystac/extensions/base.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
from abc import ABC, abstractmethod
from typing import Generic, Iterable, List, Optional, Dict, Any, Type, TypeVar, Union

import pystac
from pystac import Collection, RangeSummary, STACObject, Summaries


class SummariesExtension:
def __init__(self, collection: pystac.Collection) -> None:
summaries: Summaries

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 @@ -26,8 +28,8 @@ class PropertiesExtension(ABC):
properties: Dict[str, Any]
additional_read_properties: Optional[Iterable[Dict[str, Any]]] = None

def _get_property(self, prop_name: str, typ: Type[P] = Type[Any]) -> Optional[P]:
result: Optional[typ] = self.properties.get(prop_name)
def _get_property(self, prop_name: str, typ: Type[P]) -> Optional[P]:
result = self.properties.get(prop_name)
if result is not None:
return result
if self.additional_read_properties is not None:
Expand All @@ -46,7 +48,7 @@ def _set_property(
self.properties[prop_name] = v


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


class ExtensionManagementMixin(Generic[S], ABC):
Expand Down
2 changes: 1 addition & 1 deletion pystac/extensions/datacube.py
Original file line number Diff line number Diff line change
Expand Up @@ -386,4 +386,4 @@ class DatacubeExtensionHooks(ExtensionHooks):
)


DATACUBE_EXTENSION_HOOKS = DatacubeExtensionHooks()
DATACUBE_EXTENSION_HOOKS: ExtensionHooks = DatacubeExtensionHooks()
13 changes: 7 additions & 6 deletions pystac/extensions/eo.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,18 +272,18 @@ def bands(self) -> Optional[List[Band]]:
"""
return self._get_bands()

def _get_bands(self) -> Optional[List[Band]]:
return map_opt(
lambda bands: [Band(b) for b in bands],
self._get_property(BANDS_PROP, List[Dict[str, Any]]),
)

@bands.setter
def bands(self, v: Optional[List[Band]]) -> None:
self._set_property(
BANDS_PROP, map_opt(lambda bands: [b.to_dict() for b in bands], v)
)

def _get_bands(self) -> Optional[List[Band]]:
return map_opt(
lambda bands: [Band(b) for b in bands],
self._get_property(BANDS_PROP, List[Dict[str, Any]]),
)

@property
def cloud_cover(self) -> Optional[float]:
"""Get or sets the estimate of cloud cover as a percentage (0-100) of the
Expand Down Expand Up @@ -365,6 +365,7 @@ def bands(self) -> Optional[List[Band]]:
"""Get or sets a list of :class:`~pystac.Band` objects that represent
the available bands.
"""

return map_opt(
lambda bands: [Band(b) for b in bands],
self.summaries.get_list(BANDS_PROP, Dict[str, Any]),
Expand Down
3 changes: 2 additions & 1 deletion pystac/extensions/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ def data_type(self) -> Optional[List[FileDataType]]:
Returns:
FileDataType
"""

return map_opt(
lambda x: [FileDataType(t) for t in x],
Copy link
Member

Choose a reason for hiding this comment

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

Is it the case that mypy doesn't allow lambdas? Is there a flag for this? I find this style of short lambda useful and more readable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it allows lambdas; it's just simpler to type a function. I'm not sure you can type lambdas, but I expect it'd be a mess.

Copy link
Member

Choose a reason for hiding this comment

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

Ah right, I think Pyright is OK with inferring types in lambdas without annotations where mypy isn't...a bit of inconvenience but worth it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this may actually be an indicator that we're not adequately typing something in the second argument to map_opt. I left a comment on pystac.extensions.eo to illustrate this in more detail, but I think we may want to keep the untyped lambdas here and track down the typing issues in more detail.

I think the solution from the other comment will also solve this case and allow us to keep the untyped lambda here.

self.summaries.get_list(DATA_TYPE_PROP, str),
Expand Down Expand Up @@ -261,4 +262,4 @@ def migrate(
obj["assets"][key][CHECKSUM_PROP] = old_checksum[key]


FILE_EXTENSION_HOOKS = FileExtensionHooks()
FILE_EXTENSION_HOOKS: ExtensionHooks = FileExtensionHooks()
6 changes: 3 additions & 3 deletions pystac/extensions/hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ def schema_uri(self) -> str:

@property
@abstractmethod
def prev_extension_ids(self) -> List[str]:
"""A list of previous extension IDs (schema URIs or old short ids)
def prev_extension_ids(self) -> Set[str]:
"""A set of previous extension IDs (schema URIs or old short ids)
that should be migrated to the latest schema URI in the 'stac_extensions'
property. Override with a class attribute so that the list of previous
property. Override with a class attribute so that the set of previous
IDs is only created once.
"""
pass
Expand Down
14 changes: 7 additions & 7 deletions pystac/extensions/item_assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def __init__(self, properties: Dict[str, Any]) -> None:

@property
def title(self) -> Optional[str]:
self.properties.get(ASSET_TITLE_PROP)
return self.properties.get(ASSET_TITLE_PROP)

@title.setter
def title(self, v: Optional[str]) -> None:
Expand All @@ -38,7 +38,7 @@ def title(self, v: Optional[str]) -> None:

@property
def description(self) -> Optional[str]:
self.properties.get(ASSET_DESC_PROP)
return self.properties.get(ASSET_DESC_PROP)

@description.setter
def description(self, v: Optional[str]) -> None:
Expand All @@ -49,7 +49,7 @@ def description(self, v: Optional[str]) -> None:

@property
def media_type(self) -> Optional[str]:
self.properties.get(ASSET_TYPE_PROP)
return self.properties.get(ASSET_TYPE_PROP)

@media_type.setter
def media_type(self, v: Optional[str]) -> None:
Expand All @@ -60,7 +60,7 @@ def media_type(self, v: Optional[str]) -> None:

@property
def roles(self) -> Optional[List[str]]:
self.properties.get(ASSET_ROLES_PROP)
return self.properties.get(ASSET_ROLES_PROP)

@roles.setter
def roles(self, v: Optional[List[str]]) -> None:
Expand All @@ -78,7 +78,7 @@ def create_asset(self, href: str) -> pystac.Asset:
roles=self.roles,
properties={
k: v
for k, v in self.properties
for k, v in self.properties.items()
if k
not in set(
[
Expand All @@ -98,7 +98,7 @@ def __init__(self, collection: pystac.Collection) -> None:

@property
def item_assets(self) -> Dict[str, AssetDefinition]:
result = get_required(
result: Dict[str, Any] = get_required(
self.collection.extra_fields.get(ITEM_ASSETS_PROP), self, ITEM_ASSETS_PROP
)
return {k: AssetDefinition(v) for k, v in result.items()}
Expand Down Expand Up @@ -141,4 +141,4 @@ def migrate(
super().migrate(obj, version, info)


ITEM_ASSETS_EXTENSION_HOOKS = ItemAssetsExtensionHooks()
ITEM_ASSETS_EXTENSION_HOOKS: ExtensionHooks = ItemAssetsExtensionHooks()
2 changes: 1 addition & 1 deletion pystac/extensions/pointcloud.py
Original file line number Diff line number Diff line change
Expand Up @@ -563,4 +563,4 @@ class PointcloudExtensionHooks(ExtensionHooks):
stac_object_types: Set[pystac.STACObjectType] = set([pystac.STACObjectType.ITEM])


POINTCLOUD_EXTENSION_HOOKS = PointcloudExtensionHooks()
POINTCLOUD_EXTENSION_HOOKS: ExtensionHooks = PointcloudExtensionHooks()
2 changes: 1 addition & 1 deletion pystac/extensions/projection.py
Original file line number Diff line number Diff line change
Expand Up @@ -294,4 +294,4 @@ class ProjectionExtensionHooks(ExtensionHooks):
stac_object_types: Set[pystac.STACObjectType] = set([pystac.STACObjectType.ITEM])


PROJECTION_EXTENSION_HOOKS = ProjectionExtensionHooks()
PROJECTION_EXTENSION_HOOKS: ExtensionHooks = ProjectionExtensionHooks()
5 changes: 2 additions & 3 deletions pystac/extensions/sar.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@

import pystac
from pystac.serialization.identify import STACJSONDescription, STACVersionID
from pystac.extensions.base import ExtensionManagementMixin
from pystac.extensions.projection import ProjectionExtension
from pystac.extensions.base import ExtensionManagementMixin, PropertiesExtension
from pystac.extensions.hooks import ExtensionHooks
from pystac.utils import get_required, map_opt

Expand Down Expand Up @@ -59,7 +58,7 @@ class ObservationDirection(enum.Enum):


class SarExtension(
Generic[T], ProjectionExtension[T], ExtensionManagementMixin[pystac.Item]
Generic[T], PropertiesExtension, ExtensionManagementMixin[pystac.Item]
Copy link
Member

Choose a reason for hiding this comment

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

Nice catch

):
"""SarItemExt extends Item to add sar properties to a STAC Item.

Expand Down
2 changes: 1 addition & 1 deletion pystac/extensions/sat.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,4 +133,4 @@ class SatExtensionHooks(ExtensionHooks):
stac_object_types: Set[pystac.STACObjectType] = set([pystac.STACObjectType.ITEM])


SAT_EXTENSION_HOOKS = SatExtensionHooks()
SAT_EXTENSION_HOOKS: ExtensionHooks = SatExtensionHooks()
2 changes: 1 addition & 1 deletion pystac/extensions/scientific.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,4 +233,4 @@ class ScientificExtensionHooks(ExtensionHooks):
)


SCIENTIFIC_EXTENSION_HOOKS = ScientificExtensionHooks()
SCIENTIFIC_EXTENSION_HOOKS: ExtensionHooks = ScientificExtensionHooks()
2 changes: 1 addition & 1 deletion pystac/extensions/timestamps.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,4 +154,4 @@ class TimestampsExtensionHooks(ExtensionHooks):
stac_object_types: Set[pystac.STACObjectType] = set([pystac.STACObjectType.ITEM])


TIMESTAMPS_EXTENSION_HOOKS = TimestampsExtensionHooks()
TIMESTAMPS_EXTENSION_HOOKS: ExtensionHooks = TimestampsExtensionHooks()
2 changes: 1 addition & 1 deletion pystac/extensions/view.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,4 +195,4 @@ class ViewExtensionHooks(ExtensionHooks):
stac_object_types: Set[pystac.STACObjectType] = set([pystac.STACObjectType.ITEM])


VIEW_EXTENSION_HOOKS = ViewExtensionHooks()
VIEW_EXTENSION_HOOKS: ExtensionHooks = ViewExtensionHooks()
4 changes: 3 additions & 1 deletion pystac/stac_io.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
try:
import orjson # type: ignore
except ImportError:
orjson = None
orjson = None # type: ignore[assignment]

if TYPE_CHECKING:
from pystac.stac_object import STACObject as STACObject_Type
Expand Down Expand Up @@ -170,6 +170,7 @@ class DefaultStacIO(StacIO):
def read_text(
self, source: Union[str, "Link_Type"], *args: Any, **kwargs: Any
) -> str:
href: Optional[str]
if isinstance(source, str):
href = source
else:
Expand All @@ -193,6 +194,7 @@ def read_text_from_href(self, href: str, *args: Any, **kwargs: Any) -> str:
def write_text(
self, dest: Union[str, "Link_Type"], txt: str, *args: Any, **kwargs: Any
) -> None:
href: Optional[str]
if isinstance(dest, str):
href = dest
else:
Expand Down
2 changes: 1 addition & 1 deletion requirements-dev.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
mypy==0.790
mypy==0.812
flake8==3.8.*
black==21.4b2

Expand Down
6 changes: 6 additions & 0 deletions scripts/test
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ if [ "${BASH_SOURCE[0]}" = "${0}" ]; then
if [ "${1:-}" = "--help" ]; then
usage
else
echo
echo " -- CHECKING TYPES WITH MYPY --"
echo

mypy docs pystac setup.py tests

echo
echo " -- CHECKING TYPES WITH PYRIGHT --"
echo
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from setuptools import setup, find_packages
from glob import glob

__version__ = load_source('pystac.version', 'pystac/version.py').__version__
__version__ = load_source('pystac.version', 'pystac/version.py').__version__ # type: ignore

from os.path import (
basename,
Expand Down
6 changes: 3 additions & 3 deletions tests/extensions/test_custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def apply(self, test_prop: Optional[str]) -> None:

@property
def test_prop(self) -> Optional[str]:
self._get_property(TEST_PROP, str)
return self._get_property(TEST_PROP, str)

@test_prop.setter
def test_prop(self, v: Optional[str]) -> None:
Expand Down Expand Up @@ -139,13 +139,13 @@ def migrate(


class CustomExtensionTest(unittest.TestCase):
def setUp(self):
def setUp(self) -> None:
pystac.EXTENSION_HOOKS.add_extension_hooks(CustomExtensionHooks())

def tearDown(self) -> None:
pystac.EXTENSION_HOOKS.remove_extension_hooks(SCHEMA_URI)

# TODO: Test custom extensions and extension hooks

def test_migrates(self):
def test_migrates(self) -> None:
pass
Loading