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.

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

ENH: keepdims=True for xarray reductions #3033

Merged
merged 7 commits into from
Jun 23, 2019
Merged

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.

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
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

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

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
Co-Authored-By: Maximilian Roos <5635139+max-sixty@users.noreply.github.com>
@ScottWales
Copy link
Contributor Author

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

xarray/core/variable.py Outdated Show resolved Hide resolved
xarray/core/variable.py Outdated Show resolved Hide resolved
# 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')?

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

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
* 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)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

keepdims=True for xarray reductions
4 participants