From 320ef116cfa55f700564cab7bdaa540315ac772d Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Wed, 30 Oct 2019 22:13:57 -0400 Subject: [PATCH 1/5] type check sentinel values, using Enum pattern --- xarray/core/dataarray.py | 16 ++++++---------- xarray/core/dataset.py | 39 ++++++++++++++++++--------------------- xarray/core/utils.py | 8 ++++++++ 3 files changed, 32 insertions(+), 31 deletions(-) diff --git a/xarray/core/dataarray.py b/xarray/core/dataarray.py index 502d88f4f1f..f1194ed3892 100644 --- a/xarray/core/dataarray.py +++ b/xarray/core/dataarray.py @@ -53,7 +53,7 @@ from .formatting import format_item from .indexes import Indexes, default_indexes from .options import OPTIONS -from .utils import ReprObject, _check_inplace, either_dict_or_kwargs +from .utils import Default, ReprObject, __default, _check_inplace, either_dict_or_kwargs from .variable import ( IndexVariable, Variable, @@ -270,8 +270,6 @@ class DataArray(AbstractArray, DataWithCoords): _coarsen_cls = rolling.DataArrayCoarsen _resample_cls = resample.DataArrayResample - __default = ReprObject("") - dt = property(DatetimeAccessor) def __init__( @@ -387,18 +385,18 @@ def _replace( self, variable: Variable = None, coords=None, - name: Optional[Hashable] = __default, + name: Union[Optional[Hashable], Default] = __default, ) -> "DataArray": if variable is None: variable = self.variable if coords is None: coords = self._coords - if name is self.__default: + if name is __default: name = self.name return type(self)(variable, coords, name=name, fastpath=True) def _replace_maybe_drop_dims( - self, variable: Variable, name: Optional[Hashable] = __default + self, variable: Variable, name: Union[Optional[Hashable], Default] = __default ) -> "DataArray": if variable.dims == self.dims and variable.shape == self.shape: coords = self._coords.copy() @@ -2450,13 +2448,11 @@ def identical(self, other: "DataArray") -> bool: except (TypeError, AttributeError): return False - __default_name = object() - def _result_name(self, other: Any = None) -> Optional[Hashable]: # use the same naming heuristics as pandas: # https://github.com/ContinuumIO/blaze/issues/458#issuecomment-51936356 - other_name = getattr(other, "name", self.__default_name) - if other_name is self.__default_name or other_name == self.name: + other_name = getattr(other, "name", __default) + if other_name is __default or other_name == self.name: return self.name else: return None diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index 31efcb1d591..b1c61f47139 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -70,8 +70,10 @@ from .options import OPTIONS, _get_keep_attrs from .pycompat import dask_array_type from .utils import ( + Default, Frozen, SortedKeysDict, + __default, _check_inplace, decode_numpy_dict_values, either_dict_or_kwargs, @@ -410,7 +412,7 @@ class Dataset(Mapping, ImplementsDatasetReduce, DataWithCoords): """ _accessors: Optional[Dict[str, Any]] - _attrs: Optional[Dict[Hashable, Any]] + _attrs: Union[Dict[Hashable, Any], None] _coord_names: Set[Hashable] _dims: Dict[Hashable, int] _encoding: Optional[Dict[Hashable, Any]] @@ -856,23 +858,18 @@ def _construct_direct( obj._accessors = None return obj - __default = object() - @classmethod def _from_vars_and_coord_names(cls, variables, coord_names, attrs=None): return cls._construct_direct(variables, coord_names, attrs=attrs) - # TODO(shoyer): renable type checking on this signature when pytype has a - # good way to handle defaulting arguments to a sentinel value: - # https://github.com/python/mypy/issues/1803 - def _replace( # type: ignore + def _replace( self, variables: Dict[Hashable, Variable] = None, coord_names: Set[Hashable] = None, dims: Dict[Any, int] = None, - attrs: Optional[Dict[Hashable, Any]] = __default, - indexes: Optional[Dict[Any, pd.Index]] = __default, - encoding: Optional[dict] = __default, + attrs: Union[None, Dict[Hashable, Any], Default] = __default, + indexes: Union[Optional[Dict[Any, pd.Index]], Default] = __default, + encoding: Union[Optional[dict], Default] = __default, inplace: bool = False, ) -> "Dataset": """Fastpath constructor for internal use. @@ -890,11 +887,11 @@ def _replace( # type: ignore self._coord_names = coord_names if dims is not None: self._dims = dims - if attrs is not self.__default: + if attrs is not __default: self._attrs = attrs - if indexes is not self.__default: + if indexes is not __default: self._indexes = indexes - if encoding is not self.__default: + if encoding is not __default: self._encoding = encoding obj = self else: @@ -904,23 +901,23 @@ def _replace( # type: ignore coord_names = self._coord_names.copy() if dims is None: dims = self._dims.copy() - if attrs is self.__default: + if attrs is __default: attrs = copy.copy(self._attrs) - if indexes is self.__default: + if indexes is __default: indexes = copy.copy(self._indexes) - if encoding is self.__default: + if encoding is __default: encoding = copy.copy(self._encoding) obj = self._construct_direct( variables, coord_names, dims, attrs, indexes, encoding ) return obj - def _replace_with_new_dims( # type: ignore + def _replace_with_new_dims( self, variables: Dict[Hashable, Variable], coord_names: set = None, - attrs: Optional[Dict[Hashable, Any]] = __default, - indexes: Dict[Hashable, pd.Index] = __default, + attrs: Union[Optional[Dict[Hashable, Any]], Default] = __default, + indexes: Union[Dict[Hashable, pd.Index], Default] = __default, inplace: bool = False, ) -> "Dataset": """Replace variables with recalculated dimensions.""" @@ -929,12 +926,12 @@ def _replace_with_new_dims( # type: ignore variables, coord_names, dims, attrs, indexes, inplace=inplace ) - def _replace_vars_and_dims( # type: ignore + def _replace_vars_and_dims( self, variables: Dict[Hashable, Variable], coord_names: set = None, dims: Dict[Hashable, int] = None, - attrs: Dict[Hashable, Any] = __default, + attrs: Union[Dict[Hashable, Any], Default] = __default, inplace: bool = False, ) -> "Dataset": """Deprecated version of _replace_with_new_dims(). diff --git a/xarray/core/utils.py b/xarray/core/utils.py index 492c595a887..fb494e464af 100644 --- a/xarray/core/utils.py +++ b/xarray/core/utils.py @@ -6,6 +6,7 @@ import os.path import re import warnings +from enum import Enum from typing import ( AbstractSet, Any, @@ -701,3 +702,10 @@ def get_temp_dimname(dims: Container[Hashable], new_dim: Hashable) -> Hashable: while new_dim in dims: new_dim = "_" + str(new_dim) return new_dim + + +class Default(Enum): + sentinel = 0 + + +__default = Default.sentinel From df953721a0e93ec8bb4307f585bc5715b3c6e9fe Mon Sep 17 00:00:00 2001 From: Guido Imperiale Date: Thu, 31 Oct 2019 12:03:49 +0000 Subject: [PATCH 2/5] Code review --- xarray/core/dataarray.py | 14 +++++++------- xarray/core/dataset.py | 26 +++++++++++++------------- xarray/core/utils.py | 7 +++---- 3 files changed, 23 insertions(+), 24 deletions(-) diff --git a/xarray/core/dataarray.py b/xarray/core/dataarray.py index f1194ed3892..0216898f724 100644 --- a/xarray/core/dataarray.py +++ b/xarray/core/dataarray.py @@ -53,7 +53,7 @@ from .formatting import format_item from .indexes import Indexes, default_indexes from .options import OPTIONS -from .utils import Default, ReprObject, __default, _check_inplace, either_dict_or_kwargs +from .utils import Default, ReprObject, _default, _check_inplace, either_dict_or_kwargs from .variable import ( IndexVariable, Variable, @@ -385,18 +385,18 @@ def _replace( self, variable: Variable = None, coords=None, - name: Union[Optional[Hashable], Default] = __default, + name: Union[Optional[Hashable], Default] = _default, ) -> "DataArray": if variable is None: variable = self.variable if coords is None: coords = self._coords - if name is __default: + if name is _default: name = self.name return type(self)(variable, coords, name=name, fastpath=True) def _replace_maybe_drop_dims( - self, variable: Variable, name: Union[Optional[Hashable], Default] = __default + self, variable: Variable, name: Union[Optional[Hashable], Default] = _default ) -> "DataArray": if variable.dims == self.dims and variable.shape == self.shape: coords = self._coords.copy() @@ -436,7 +436,7 @@ def _to_temp_dataset(self) -> Dataset: return self._to_dataset_whole(name=_THIS_ARRAY, shallow_copy=False) def _from_temp_dataset( - self, dataset: Dataset, name: Hashable = __default + self, dataset: Dataset, name: Hashable = _default ) -> "DataArray": variable = dataset._variables.pop(_THIS_ARRAY) coords = dataset._variables @@ -2451,8 +2451,8 @@ def identical(self, other: "DataArray") -> bool: def _result_name(self, other: Any = None) -> Optional[Hashable]: # use the same naming heuristics as pandas: # https://github.com/ContinuumIO/blaze/issues/458#issuecomment-51936356 - other_name = getattr(other, "name", __default) - if other_name is __default or other_name == self.name: + other_name = getattr(other, "name", _default) + if other_name is _default or other_name == self.name: return self.name else: return None diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index b1c61f47139..c61d0e219fc 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -73,7 +73,7 @@ Default, Frozen, SortedKeysDict, - __default, + _default, _check_inplace, decode_numpy_dict_values, either_dict_or_kwargs, @@ -867,9 +867,9 @@ def _replace( variables: Dict[Hashable, Variable] = None, coord_names: Set[Hashable] = None, dims: Dict[Any, int] = None, - attrs: Union[None, Dict[Hashable, Any], Default] = __default, - indexes: Union[Optional[Dict[Any, pd.Index]], Default] = __default, - encoding: Union[Optional[dict], Default] = __default, + attrs: Union[None, Dict[Hashable, Any], Default] = _default, + indexes: Union[Optional[Dict[Any, pd.Index]], Default] = _default, + encoding: Union[Optional[dict], Default] = _default, inplace: bool = False, ) -> "Dataset": """Fastpath constructor for internal use. @@ -887,11 +887,11 @@ def _replace( self._coord_names = coord_names if dims is not None: self._dims = dims - if attrs is not __default: + if not isinstance(attrs, Default): self._attrs = attrs - if indexes is not __default: + if not isinstance(indexes, Default): self._indexes = indexes - if encoding is not __default: + if not isinstance(encoding, Default): self._encoding = encoding obj = self else: @@ -901,11 +901,11 @@ def _replace( coord_names = self._coord_names.copy() if dims is None: dims = self._dims.copy() - if attrs is __default: + if attrs is _default: attrs = copy.copy(self._attrs) - if indexes is __default: + if indexes is _default: indexes = copy.copy(self._indexes) - if encoding is __default: + if encoding is _default: encoding = copy.copy(self._encoding) obj = self._construct_direct( variables, coord_names, dims, attrs, indexes, encoding @@ -916,8 +916,8 @@ def _replace_with_new_dims( self, variables: Dict[Hashable, Variable], coord_names: set = None, - attrs: Union[Optional[Dict[Hashable, Any]], Default] = __default, - indexes: Union[Dict[Hashable, pd.Index], Default] = __default, + attrs: Union[Optional[Dict[Hashable, Any]], Default] = _default, + indexes: Union[Dict[Hashable, pd.Index], Default] = _default, inplace: bool = False, ) -> "Dataset": """Replace variables with recalculated dimensions.""" @@ -931,7 +931,7 @@ def _replace_vars_and_dims( variables: Dict[Hashable, Variable], coord_names: set = None, dims: Dict[Hashable, int] = None, - attrs: Union[Dict[Hashable, Any], Default] = __default, + attrs: Union[Dict[Hashable, Any], Default] = _default, inplace: bool = False, ) -> "Dataset": """Deprecated version of _replace_with_new_dims(). diff --git a/xarray/core/utils.py b/xarray/core/utils.py index fb494e464af..4a17f453b3a 100644 --- a/xarray/core/utils.py +++ b/xarray/core/utils.py @@ -6,7 +6,6 @@ import os.path import re import warnings -from enum import Enum from typing import ( AbstractSet, Any, @@ -704,8 +703,8 @@ def get_temp_dimname(dims: Container[Hashable], new_dim: Hashable) -> Hashable: return new_dim -class Default(Enum): - sentinel = 0 +class Default: + pass -__default = Default.sentinel +_default = Default() From 403d2983b580acfc9a8be3bd9f6c7923258f0238 Mon Sep 17 00:00:00 2001 From: Guido Imperiale Date: Thu, 31 Oct 2019 12:15:54 +0000 Subject: [PATCH 3/5] Code review --- xarray/core/dataset.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index c61d0e219fc..7041e87b9ba 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -412,7 +412,7 @@ class Dataset(Mapping, ImplementsDatasetReduce, DataWithCoords): """ _accessors: Optional[Dict[str, Any]] - _attrs: Union[Dict[Hashable, Any], None] + _attrs: Optional[Dict[Hashable, Any]] _coord_names: Set[Hashable] _dims: Dict[Hashable, int] _encoding: Optional[Dict[Hashable, Any]] @@ -867,9 +867,9 @@ def _replace( variables: Dict[Hashable, Variable] = None, coord_names: Set[Hashable] = None, dims: Dict[Any, int] = None, - attrs: Union[None, Dict[Hashable, Any], Default] = _default, - indexes: Union[Optional[Dict[Any, pd.Index]], Default] = _default, - encoding: Union[Optional[dict], Default] = _default, + attrs: Union[Dict[Hashable, Any], None, Default] = _default, + indexes: Union[Dict[Any, pd.Index], None, Default] = _default, + encoding: Union[dict, None, Default] = _default, inplace: bool = False, ) -> "Dataset": """Fastpath constructor for internal use. @@ -916,8 +916,8 @@ def _replace_with_new_dims( self, variables: Dict[Hashable, Variable], coord_names: set = None, - attrs: Union[Optional[Dict[Hashable, Any]], Default] = _default, - indexes: Union[Dict[Hashable, pd.Index], Default] = _default, + attrs: Union[Dict[Hashable, Any], None, Default] = _default, + indexes: Union[Dict[Hashable, pd.Index], None, Default] = _default, inplace: bool = False, ) -> "Dataset": """Replace variables with recalculated dimensions.""" @@ -931,7 +931,7 @@ def _replace_vars_and_dims( variables: Dict[Hashable, Variable], coord_names: set = None, dims: Dict[Hashable, int] = None, - attrs: Union[Dict[Hashable, Any], Default] = _default, + attrs: Union[Dict[Hashable, Any], None, Default] = _default, inplace: bool = False, ) -> "Dataset": """Deprecated version of _replace_with_new_dims(). From 52790328e34b7531d9a419aa737eb32689a3cba2 Mon Sep 17 00:00:00 2001 From: Guido Imperiale Date: Thu, 31 Oct 2019 12:18:54 +0000 Subject: [PATCH 4/5] Code review --- xarray/core/dataarray.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xarray/core/dataarray.py b/xarray/core/dataarray.py index 0216898f724..82be3989b27 100644 --- a/xarray/core/dataarray.py +++ b/xarray/core/dataarray.py @@ -385,7 +385,7 @@ def _replace( self, variable: Variable = None, coords=None, - name: Union[Optional[Hashable], Default] = _default, + name: Union[Hashable, None, Default] = _default, ) -> "DataArray": if variable is None: variable = self.variable @@ -396,7 +396,7 @@ def _replace( return type(self)(variable, coords, name=name, fastpath=True) def _replace_maybe_drop_dims( - self, variable: Variable, name: Union[Optional[Hashable], Default] = _default + self, variable: Variable, name: Union[Hashable, None, Default] = _default ) -> "DataArray": if variable.dims == self.dims and variable.shape == self.shape: coords = self._coords.copy() From 92edcbb1841503855b37545c753d91f9c56eabcc Mon Sep 17 00:00:00 2001 From: Guido Imperiale Date: Thu, 31 Oct 2019 15:01:55 +0000 Subject: [PATCH 5/5] Code review --- xarray/core/dataset.py | 12 ++++++------ xarray/core/utils.py | 8 +++++--- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index 7041e87b9ba..6e94d35df40 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -887,12 +887,12 @@ def _replace( self._coord_names = coord_names if dims is not None: self._dims = dims - if not isinstance(attrs, Default): - self._attrs = attrs - if not isinstance(indexes, Default): - self._indexes = indexes - if not isinstance(encoding, Default): - self._encoding = encoding + if attrs is not _default: + self._attrs = attrs # type: ignore # FIXME need mypy 0.750 + if indexes is not _default: + self._indexes = indexes # type: ignore # FIXME need mypy 0.750 + if encoding is not _default: + self._encoding = encoding # type: ignore # FIXME need mypy 0.750 obj = self else: if variables is None: diff --git a/xarray/core/utils.py b/xarray/core/utils.py index 4a17f453b3a..6681375c18e 100644 --- a/xarray/core/utils.py +++ b/xarray/core/utils.py @@ -6,6 +6,7 @@ import os.path import re import warnings +from enum import Enum from typing import ( AbstractSet, Any, @@ -703,8 +704,9 @@ def get_temp_dimname(dims: Container[Hashable], new_dim: Hashable) -> Hashable: return new_dim -class Default: - pass +# Singleton type, as per https://github.com/python/typing/pull/240 +class Default(Enum): + token = 0 -_default = Default() +_default = Default.token