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

flox: don't set fill_value where possible #9433

Merged
merged 10 commits into from
Sep 18, 2024

Conversation

dcherian
Copy link
Contributor

@dcherian dcherian commented Sep 5, 2024

Closes #8090
Closes #8206
Closes #9398

  • Tests added
  • User visible changes (including notable bug fixes) are documented in whats-new.rst

@dcherian dcherian marked this pull request as draft September 5, 2024 14:41
@dcherian dcherian marked this pull request as ready for review September 5, 2024 14:54
@@ -10387,7 +10387,7 @@ def groupby(
* letters (letters) object 16B 'a' 'b'
Dimensions without coordinates: y
Data variables:
foo (letters, y) float64 48B 9.0 11.0 13.0 9.0 11.0 13.0
foo (letters, y) int64 48B 9 11 13 9 11 13
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an improvement, since dtype is preserved now

@@ -787,14 +787,12 @@ def _maybe_restore_empty_groups(self, combined):
"""Our index contained empty groups (e.g., from a resampling or binning). If we
reduced on that dimension, we want to restore the full index.
"""
from xarray.groupers import BinGrouper, TimeResampler

has_missing_groups = (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice generalization.

isinstance(grouper.grouper, BinGrouper | TimeResampler)
and grouper.name in combined.dims
):
if has_missing_groups and grouper.name in combined._indexes:
Copy link
Contributor Author

@dcherian dcherian Sep 5, 2024

Choose a reason for hiding this comment

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

The switch from .dims to ._indexes is a bugfix. It was caught by existing multiple grouper tests

* main: (26 commits)
  Forbid modifying names of DataTree objects with parents (pydata#9494)
  DAS-2155 - Merge datatree documentation into main docs. (pydata#9033)
  Make illegal path-like variable names when constructing a DataTree from a Dataset (pydata#9378)
  Ensure TreeNode doesn't copy in-place (pydata#9482)
  `open_groups` for zarr backends (pydata#9469)
  Update pyproject.toml (pydata#9484)
  New whatsnew section (pydata#9483)
  Release notes for v2024.09.0 (pydata#9480)
  Fix `DataTree.coords.__setitem__` by adding `DataTreeCoordinates` class (pydata#9451)
  Rename DataTree's "ds" and "data" to "dataset" (pydata#9476)
  Update DataTree repr to indicate inheritance (pydata#9470)
  Bump pypa/gh-action-pypi-publish in the actions group (pydata#9460)
  Repo checker (pydata#9450)
  Add days_in_year and decimal_year to dt accessor (pydata#9105)
  remove parent argument from DataTree.__init__ (pydata#9465)
  Fix inheritance in DataTree.copy() (pydata#9457)
  Implement `DataTree.__delitem__` (pydata#9453)
  Add ASV for datatree.from_dict (pydata#9459)
  Make the first argument in DataTree.from_dict positional only (pydata#9446)
  Fix typos across the code, doc and comments (pydata#9443)
  ...
* main:
  Opt out of floor division for float dtype time encoding (pydata#9497)
  fixed formatting for whats-new (pydata#9493)
@dcherian dcherian marked this pull request as draft September 17, 2024 03:36
@dcherian dcherian marked this pull request as ready for review September 17, 2024 15:36
@dcherian dcherian added the plan to merge Final call for comments label Sep 17, 2024
@dcherian
Copy link
Contributor Author

This is a pretty internal fix and hard to review, so I'll merge in a day or so.

@dcherian dcherian merged commit e313853 into pydata:main Sep 18, 2024
28 checks passed
@dcherian dcherian deleted the flox-preserve-dtype branch September 18, 2024 18:31
hollymandel pushed a commit to hollymandel/xarray that referenced this pull request Sep 23, 2024
* flox: don't set fill_value where possible

Closes pydata#8090
Closes pydata#8206
Closes pydata#9398

* Update doctest

* Fix test

* fix test

* Test for flox >= 0.9.12

* fix whats-new
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plan to merge Final call for comments topic-groupby
Projects
None yet
1 participant