-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Document that Coarsen accepts coord func as callable #7981
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
base: main
Are you sure you want to change the base?
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 | ||||
---|---|---|---|---|---|---|
|
@@ -61,14 +61,17 @@ | |||||
if TYPE_CHECKING: | ||||||
from xarray.core.parallelcompat import ChunkManagerEntrypoint | ||||||
from xarray.core.types import ( | ||||||
CoarsenBoundaryOptions, | ||||||
Dims, | ||||||
ErrorOptionsWithWarn, | ||||||
PadModeOptions, | ||||||
PadReflectOptions, | ||||||
QuantileMethods, | ||||||
SideOptions, | ||||||
T_Variable, | ||||||
) | ||||||
|
||||||
|
||||||
NON_NANOSECOND_WARNING = ( | ||||||
"Converting non-nanosecond precision {case} values to nanosecond precision. " | ||||||
"This behavior can eventually be relaxed in xarray, as it is an artifact from " | ||||||
|
@@ -2494,10 +2497,29 @@ def rolling_window( | |||||
) | ||||||
|
||||||
def coarsen( | ||||||
self, windows, func, boundary="exact", side="left", keep_attrs=None, **kwargs | ||||||
self, | ||||||
windows: Mapping[Any, int], | ||||||
func: str | Callable, | ||||||
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.
Suggested change
|
||||||
boundary: CoarsenBoundaryOptions = "exact", | ||||||
side: SideOptions | Mapping[Any, SideOptions] = "left", | ||||||
keep_attrs=None, | ||||||
**kwargs, | ||||||
): | ||||||
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. While at it you could add a return type. |
||||||
""" | ||||||
Apply reduction function. | ||||||
|
||||||
Parameters | ||||||
---------- | ||||||
windows : mapping of hashable to int | ||||||
A mapping from the name of the dimension to create the coarsened block along (e.g. `time`) to the size of | ||||||
the coarsened block. | ||||||
func : function or str name of function | ||||||
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.
Suggested change
|
||||||
Function to use to reduce the values of one block down along one or more axes. | ||||||
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. Maybe mention the signature of the function. Also, I never know from which "pool" you can choose the str names, maybe this can be documented better? (Same for the other docstrings) 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.
Yeah this seems like an issue. It's currently literally just any function in the internal 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. Maybe we could add a public variable in the duck_arr_ops module that collects valid reduction names and add a reference to it? |
||||||
boundary : {"exact", "trim", "pad"} | ||||||
If 'exact', a ValueError will be raised if dimension size is not a | ||||||
multiple of window size. If 'trim', the excess indexes are trimmed. | ||||||
If 'pad', NA will be padded. | ||||||
side : 'left' or 'right' or mapping from dimension to 'left' or 'right' | ||||||
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. Add |
||||||
""" | ||||||
windows = {k: v for k, v in windows.items() if k in self.dims} | ||||||
|
||||||
|
@@ -2514,12 +2536,18 @@ def coarsen( | |||||
|
||||||
reshaped, axes = self.coarsen_reshape(windows, boundary, side) | ||||||
if isinstance(func, str): | ||||||
name = func | ||||||
func = getattr(duck_array_ops, name, None) | ||||||
if func is None: | ||||||
raise NameError(f"{name} is not a valid method.") | ||||||
try: | ||||||
callable_func = getattr(duck_array_ops, func) | ||||||
except AttributeError: | ||||||
raise NameError( | ||||||
f"{func} is not a valid xarray reduction method, so cannot be used to coarsen" | ||||||
) | ||||||
else: | ||||||
callable_func = func | ||||||
|
||||||
return self._replace(data=func(reshaped, axis=axes, **kwargs), attrs=_attrs) | ||||||
return self._replace( | ||||||
data=callable_func(reshaped, axis=axes, **kwargs), attrs=_attrs | ||||||
) | ||||||
|
||||||
def coarsen_reshape(self, windows, boundary, side): | ||||||
""" | ||||||
|
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.
Also probably more readable that way because you don't have 4 times "function" in one sentence.
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.
I think we should add an explicit list of function names