Skip to content
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.

Sign up for GitHub

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

Jump to bottom

ENH: keepdims=True for xarray reductions #3033

Merged
merged 7 commits into from
Jun 23, 2019
Merged

ENH: keepdims=True for xarray reductions #3033

merged 7 commits into from
Jun 23, 2019

Conversation

ScottWales
Copy link
Contributor

Add new option keepdims to xarray reduce operations, following the behaviour of Numpy.

keepdims may be passed to reductions on either Datasets or DataArrays, and will result in the reduced dimensions being still present in the output with size 1.

Coordinates that depend on the reduced dimensions will be removed from the Dataset/DataArray

The name keepdims is used here to be consistent with Numpy, keep_dims was an alternative name proposed in #2170.

The functionality has only been added to Variable.reduce(), Dataarray.reduce() and DataSet.reduce() to start off with, it is not implemented for GroupBy, Rolling or Resample.

ENH: keepdims=True for xarray reductions
6ba9800 
Addresses pydata#2170

Add new option `keepdims` to xarray reduce operations, following the
behaviour of Numpy.

`keepdims` may be passed to reductions on either Datasets or DataArrays,
and will result in the reduced dimensions being still present in the
output with size 1.

Coordinates that depend on the reduced dimensions will be removed from
the Dataset/DataArray
max-sixty
max-sixty reviewed Jun 19, 2019
View reviewed changes
Copy link
Collaborator

@max-sixty max-sixty left a comment

Choose a reason for hiding this comment

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

This looks great! Thanks @ScottWales

One question re the defaults

xarray/core/variable.py Show resolved Hide resolved
@pep8speaks
Copy link

pep8speaks commented Jun 19, 2019 •
edited
Loading

Hello @ScottWales! 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 2019-06-20 04:55:25 UTC

max-sixty
max-sixty approved these changes Jun 19, 2019
View reviewed changes
Copy link
Collaborator

@max-sixty max-sixty left a comment

Choose a reason for hiding this comment

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

I added a couple of tweaks re the default - let me know your thoughts

Then I'll merge later unless anyone else has comments

Thanks a lot @ScottWales ! Happy to have you as a contributor

xarray/core/dataarray.py Outdated Show resolved Hide resolved
xarray/core/dataarray.py Show resolved Hide resolved
xarray/core/dataset.py Outdated Show resolved Hide resolved
xarray/core/variable.py Outdated Show resolved Hide resolved
@ScottWales
Copy link
Contributor Author

Thanks for the review @max-sixty, happy to help out

shoyer
shoyer reviewed Jun 20, 2019
View reviewed changes
xarray/core/variable.py Outdated Show resolved Hide resolved
xarray/core/variable.py Outdated Show resolved Hide resolved
shoyer
shoyer approved these changes Jun 21, 2019
View reviewed changes
xarray/core/variable.py
# Insert np.newaxis for removed dims
slices = tuple(np.newaxis if i in removed_axes else
slice(None, None) for i in range(self.ndim))
if getattr(data, 'shape', None) is None:
Copy link
Member

Choose a reason for hiding this comment

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

I think this would be equivalent to just hasattr(data, 'shape')?

xarray/core/variable.py
if keepdims:
# Insert np.newaxis for removed dims
slices = tuple(np.newaxis if i in removed_axes else
slice(None, None) for i in range(self.ndim))
Copy link
Member

@shoyer shoyer Jun 21, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Just FYI, slice(None) is equivalent to slice(None, None)

@shoyer shoyer merged commit ff41988 into pydata:master Jun 23, 2019
@shoyer
Copy link
Member

shoyer commented Jun 23, 2019

Thanks @ScottWales!

dcherian added a commit to dcherian/xarray that referenced this pull request Jun 24, 2019
@dcherian
Merge branch 'master' into fix/bounds_encode_2
f187ca1 
* master: (31 commits)
  Add quantile method to GroupBy (pydata#2828)
  rolling_exp (nee ewm) (pydata#2650)
  Ensure explicitly indexed arrays are preserved (pydata#3027)
  add back dask-dev tests (pydata#3025)
  ENH: keepdims=True for xarray reductions (pydata#3033)
  Revert cmap fix (pydata#3038)
  Add "errors" keyword argument to drop() and drop_dims() (pydata#2994) (pydata#3028)
  More consistency checks (pydata#2859)
  Check types in travis (pydata#3024)
  Update issue templates (pydata#3019)
  Add pytest markers to avoid warnings (pydata#3023)
  Feature/merge errormsg (pydata#2971)
  More support for missing_value. (pydata#2973)
  Use flake8 rather than pycodestyle (pydata#3010)
  Pandas labels deprecation (pydata#3016)
  Pytest capture uses match, not message (pydata#3011)
  dask-dev tests to allowed failures in travis (pydata#3014)
  Fix 'to_masked_array' computing dask arrays twice (pydata#3006)
  str accessor (pydata#2991)
  fix safe_cast_to_index (pydata#3001)
  ...
@eric-czech eric-czech mentioned this pull request Aug 23, 2020
Closed
@JulianGiles JulianGiles mentioned this pull request Oct 16, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@max-sixty max-sixty max-sixty approved these changes

@shoyer shoyer shoyer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

keepdims=True for xarray reductions
4 participants
@ScottWales @pep8speaks @shoyer @max-sixty