-
-
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
Add DataArrayCoarsen.reduce and DatasetCoarsen.reduce methods #4939
Conversation
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.
Looks good, missing the doc entries.
@@ -871,14 +873,48 @@ def wrapped_func(self, **kwargs): | |||
|
|||
return wrapped_func | |||
|
|||
def reduce(self, func: Callable, **kwargs): | |||
"""Reduce the items in this group by applying `func` along some |
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.
"""Reduce the items in this group by applying `func` along some | |
"""Reduce the items in this group by applying ``func`` along some |
I think docstrings are in rst format?
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 went back and forth on this, and eventually opted to keep things as they were. This is consistent with the other docstrings for rolling and coarsening objects. I don't have a strong opinion though.
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.
One backtick seems to render as italics and two as code block. In text form I also like the single backticks more. I am fine to leave as is.
Thanks for the review @mathause! Let me know if things look good from your end now. |
Hello @spencerkclark! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2021-02-23 15:08:14 UTC |
Co-authored-by: Mathias Hauser <mathause@users.noreply.github.com>
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.
LGTM!
@@ -871,14 +873,48 @@ def wrapped_func(self, **kwargs): | |||
|
|||
return wrapped_func | |||
|
|||
def reduce(self, func: Callable, **kwargs): | |||
"""Reduce the items in this group by applying `func` along some |
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.
One backtick seems to render as italics and two as code block. In text form I also like the single backticks more. I am fine to leave as is.
Co-authored-by: Mathias Hauser <mathause@users.noreply.github.com>
Co-authored-by: Mathias Hauser <mathause@users.noreply.github.com>
* upstream/master: (46 commits) pin netCDF4=1.5.3 in min-all-deps (pydata#4982) fix matplotlib errors for single level discrete colormaps (pydata#4256) Adapt exception handling in CFTimeIndex.__sub__ and __rsub__ (pydata#5006) Update options.py (pydata#5000) Adjust tests to use updated pandas syntax for offsets (pydata#4537) add a combine_attrs parameter to Dataset.merge (pydata#4895) Support for dask.graph_manipulation (pydata#4965) raise on passing axis to Dataset.reduce methods (pydata#4940) Whatsnew for 0.17.1 (pydata#4963) Refinements to how-to-release (pydata#4964) DOC: add example for reindex (pydata#4956) DOC: rm np import (pydata#4949) Add 0.17.0 release notes (pydata#4953) document update as inplace (pydata#4932) bump the dependencies (pydata#4942) Upstream CI: limit runtime (pydata#4946) typing for numpy 1.20 (pydata#4878) Use definition of DTypeLike from Numpy if found (pydata#4941) autoupdate mypy (pydata#4943) Add DataArrayCoarsen.reduce and DatasetCoarsen.reduce methods (pydata#4939) ...
As suggested by @dcherian, this was quite similar to
rolling
; it was useful in particular to follow how the tests were implemented there.pre-commit run --all-files
whats-new.rst
api.rst