Skip to content

don't type check __getattr__ #4616

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

Closed
wants to merge 1 commit into from
Closed
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
5 changes: 5 additions & 0 deletions doc/whats-new.rst
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,11 @@ Internal Changes
By `Yash Saboo <https://github.com/yashsaboo>`_, `Nirupam K N
<https://github.com/Nirupamkn>`_ and `Mathias Hauser
<https://github.com/mathause>`_.
- Added ``@no_type_check`` to ``__getattr__`` of :py:class:`Dataset` and :py:class:`DataArray`.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
- Added ``@no_type_check`` to ``__getattr__`` of :py:class:`Dataset` and :py:class:`DataArray`.
- Added ``@no_type_check`` to ``__getattr__`` of :py:class:`Dataset` and :py:class:`DataArray`.

This can help to identify missing methods but may lead to typing errors in downstream libraries.
The recommendation in this case is to use the index operator when accessing variables or
coordinates, e.g. ``da["longitude"]``, and to add ``#type: ignore`` for accessors, e.g.
``da.geo # type: ignore`` (:issue:`4601`). By `Mathias Hauser <https://github.com/mathause>`_.

.. _whats-new.0.16.1:

Expand Down
62 changes: 55 additions & 7 deletions xarray/core/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
Tuple,
TypeVar,
Union,
no_type_check,
)

import numpy as np
Expand Down Expand Up @@ -218,6 +219,7 @@ def _item_sources(self) -> List[Mapping[Hashable, Any]]:
"""List of places to look-up items for key-autocompletion"""
return []

@no_type_check # so missing methods raise a type error
def __getattr__(self, name: str) -> Any:
if name not in {"__dict__", "__setstate__"}:
# this avoids an infinite loop when pickle looks for the
Expand Down Expand Up @@ -365,7 +367,7 @@ def squeeze(
numpy.squeeze
"""
dims = get_squeeze_dims(self, dim, axis)
return self.isel(drop=drop, **{d: 0 for d in dims})
return self.isel({d: 0 for d in dims}, drop=drop)

def get_index(self, key: Hashable) -> pd.Index:
"""Get an index for a dimension, with fall-back to a default RangeIndex"""
Expand Down Expand Up @@ -693,7 +695,7 @@ def groupby(self, group, squeeze: bool = True, restore_coord_dims: bool = None):
f"`squeeze` must be True or False, but {squeeze} was supplied"
)

return self._groupby_cls(
return self._groupby_cls( # type: ignore
self, group, squeeze=squeeze, restore_coord_dims=restore_coord_dims
)

Expand Down Expand Up @@ -756,7 +758,7 @@ def groupby_bins(
----------
.. [1] http://pandas.pydata.org/pandas-docs/stable/generated/pandas.cut.html
"""
return self._groupby_cls(
return self._groupby_cls( # type: ignore
self,
group,
squeeze=squeeze,
Expand Down Expand Up @@ -787,7 +789,7 @@ def weighted(self, weights):
Missing values can be replaced by ``weights.fillna(0)``.
"""

return self._weighted_cls(self, weights)
return self._weighted_cls(self, weights) # type: ignore

def rolling(
self,
Expand Down Expand Up @@ -861,7 +863,7 @@ def rolling(
"""

dim = either_dict_or_kwargs(dim, window_kwargs, "rolling")
return self._rolling_cls(
return self._rolling_cls( # type: ignore
self, dim, min_periods=min_periods, center=center, keep_attrs=keep_attrs
)

Expand Down Expand Up @@ -972,7 +974,7 @@ def coarsen(
keep_attrs = _get_keep_attrs(default=False)

dim = either_dict_or_kwargs(dim, window_kwargs, "coarsen")
return self._coarsen_cls(
return self._coarsen_cls( # type: ignore
self,
dim,
boundary=boundary,
Expand Down Expand Up @@ -1139,7 +1141,7 @@ def resample(
group = DataArray(
dim_coord, coords=dim_coord.coords, dims=dim_coord.dims, name=RESAMPLE_DIM
)
resampler = self._resample_cls(
resampler = self._resample_cls( # type: ignore
self,
group=group,
dim=dim_name,
Expand Down Expand Up @@ -1376,6 +1378,52 @@ def __getitem__(self, value):
# implementations of this class should implement this method
raise NotImplementedError()

def isel(
self,
indexers: Mapping[Hashable, Any] = None,
drop: bool = False,
missing_dims: str = "raise",
**indexers_kwargs: Any,
):
# implementations of this class should implement this method / for type checking
raise NotImplementedError()

@property
def dims(self):
# implementations of this class should implement this method / for type checking
raise NotImplementedError()

@property
def indexes(self):
# implementations of this class should implement this method / for type checking
raise NotImplementedError()

@property
def sizes(self):
# implementations of this class should implement this method / for type checking
raise NotImplementedError()

def copy(self, deep=True, data=None):
# implementations of this class should implement this method / for type checking
raise NotImplementedError()

@property
def encoding(self) -> Dict:
# implementations of this class should implement this method / for type checking
raise NotImplementedError()

def reindex(
self,
indexers: Mapping[Hashable, Any] = None,
method: str = None,
tolerance=None,
copy: bool = True,
fill_value: Any = dtypes.NA,
**indexers_kwargs: Any,
):
# implementations of this class should implement this method / for type checking
raise NotImplementedError()


def full_like(other, fill_value, dtype: DTypeLike = None):
"""Return a new object with the same shape and type as a given object.
Expand Down
9 changes: 3 additions & 6 deletions xarray/core/dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -3784,12 +3784,8 @@ def merge(
)
return self._replace(**merge_result._asdict())

def _assert_all_in_dataset(
self, names: Iterable[Hashable], virtual_okay: bool = False
) -> None:
def _assert_all_in_dataset(self, names: Iterable[Hashable]) -> None:
bad_names = set(names) - set(self._variables)
if virtual_okay:
bad_names -= self.virtual_variables
if bad_names:
raise ValueError(
"One or more of the specified variables "
Expand Down Expand Up @@ -6132,7 +6128,8 @@ def polyfit(
# deficient ranks nor does it output the "full" info (issue dask/dask#6516)
skipna_da = True
elif skipna is None:
skipna_da = np.any(da.isnull())
# no type checking because ops are injected
skipna_da = np.any(da.isnull()) # type: ignore

dims_to_stack = [dimname for dimname in da.dims if dimname != dim]
stacked_coords: Dict[Hashable, DataArray] = {}
Expand Down
3 changes: 2 additions & 1 deletion xarray/core/weighted.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,8 @@ def _sum_of_weights(
""" Calculate the sum of weights, accounting for missing values """

# we need to mask data values that are nan; else the weights are wrong
mask = da.notnull()
# no type checking because ops are injected
mask = da.notnull() # type: ignore
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this mean that any library downstream that runs .notnull will get a type error? I worry that'll cause a lot of false positives.

What do others think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nix this! @keewis has handled, too fast for me #4618


# bool -> int, because ``xr.dot([True, True], [True, True])`` -> True
# (and not 2); GH4074
Expand Down