-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
rolling_exp: keep_attrs and typing #4592
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,17 @@ | ||
from typing import TYPE_CHECKING, Generic, Hashable, Mapping, Optional, TypeVar | ||
|
||
import numpy as np | ||
|
||
from .options import _get_keep_attrs | ||
from .pdcompat import count_not_none | ||
from .pycompat import is_duck_dask_array | ||
|
||
if TYPE_CHECKING: | ||
from .dataarray import DataArray # noqa: F401 | ||
from .dataset import Dataset # noqa: F401 | ||
|
||
T_DSorDA = TypeVar("T_DSorDA", "DataArray", "Dataset") | ||
|
||
|
||
def _get_alpha(com=None, span=None, halflife=None, alpha=None): | ||
# pandas defines in terms of com (converting to alpha in the algo) | ||
|
@@ -56,7 +65,7 @@ def _get_center_of_mass(comass, span, halflife, alpha): | |
return float(comass) | ||
|
||
|
||
class RollingExp: | ||
class RollingExp(Generic[T_DSorDA]): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great idea with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes that took quite a while to figure out but it doesn't work without it as the Type of |
||
""" | ||
Exponentially-weighted moving window object. | ||
Similar to EWM in pandas | ||
|
@@ -78,16 +87,28 @@ class RollingExp: | |
RollingExp : type of input argument | ||
""" | ||
|
||
def __init__(self, obj, windows, window_type="span"): | ||
self.obj = obj | ||
def __init__( | ||
self, | ||
obj: T_DSorDA, | ||
windows: Mapping[Hashable, int], | ||
window_type: str = "span", | ||
): | ||
self.obj: T_DSorDA = obj | ||
dim, window = next(iter(windows.items())) | ||
self.dim = dim | ||
self.alpha = _get_alpha(**{window_type: window}) | ||
|
||
def mean(self): | ||
def mean(self, keep_attrs: Optional[bool] = None) -> T_DSorDA: | ||
""" | ||
Exponentially weighted moving average | ||
|
||
Parameters | ||
---------- | ||
keep_attrs : bool, default: None | ||
If True, the attributes (``attrs``) will be copied from the original | ||
object to the new one. If False, the new object will be returned | ||
without attributes. If None uses the global default. | ||
|
||
Examples | ||
-------- | ||
>>> da = xr.DataArray([1, 1, 2, 2, 2], dims="x") | ||
|
@@ -97,4 +118,9 @@ def mean(self): | |
Dimensions without coordinates: x | ||
""" | ||
|
||
return self.obj.reduce(move_exp_nanmean, dim=self.dim, alpha=self.alpha) | ||
if keep_attrs is None: | ||
keep_attrs = _get_keep_attrs(default=True) | ||
|
||
return self.obj.reduce( | ||
move_exp_nanmean, dim=self.dim, alpha=self.alpha, keep_attrs=keep_attrs | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this different from
DataWithCoords
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
works as well. A bit to my surprise as
DataWithCoords
does not implementreduce
. But mypy does not throw an error here:not sure why not...
(Also I am not entirely sure what the difference is between
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is odd it doesn't throw an error...
On the second case, I think — not completely sure — that
T_Parent
is generic with an "upper bound" ofParent
, so any function that takesT_Parent
will return the same type.Whereas a function that takes
T_Children
could return either ofChildA
orChildB
regardless of what's passed in.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And your call on whether we use
DataWithCoords
or theTypeVar
, but I think if we do the latter we could find a clearer name :)I would marginally vote to go with
DataWithCoords
given we do that elsewhere but totally open on trying another approachThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think both would need to be
TypeVar
s. I I'll stick toT_DSorDA
asDataWithCoords
does not implementreduce
.T_DSorDA
is already used at some places (map_blocks
), e.g.:xarray/xarray/core/parallel.py
Line 34 in 5296ed1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I didn't know that, thanks. I wasn't a fan of the name, but we can change them together another time if others agree.