From 5d9750e76affc41301cf645fcf39d61cde371505 Mon Sep 17 00:00:00 2001 From: Shurong Cao <170531907+CAOShurong@users.noreply.github.com> Date: Wed, 26 Aug 2026 16:02:07 +0800 Subject: [PATCH 1/4] Fix idxmax/idxmin silently promoting integer label coordinate dtype to float64 (GH#7527) Previously, on floating-point data the integer coordinate labels of idxmax/idxmin results were silently cast to float64 even for fully-valid reduction slices, because _calc_idxminmax called res.where unconditionally. Now .where only runs for non-chunked inputs when a slice is actually all-NaN, so valid slices keep their coordinate dtype. Chunked (dask) inputs keep the original delayed .where to avoid forcing an extra compute(). Closes #7527 --- doc/whats-new.rst | 9 +++++++++ xarray/computation/computation.py | 15 +++++++++++++-- xarray/core/dataset.py | 8 ++++---- xarray/tests/test_dataarray.py | 30 ++++++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 6 deletions(-) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index d1505bfa081..96141c9ba89 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -105,6 +105,15 @@ Bug Fixes sparse variables sharing the same dims (:issue:`4007`). By `patnr `_. +- :py:meth:`DataArray.idxmax` / :py:meth:`DataArray.idxmin` and the + :py:class:`Dataset` equivalents no longer promote the dtype of the returned + label coordinate when no ``fill_value`` is needed. Previously, on floating-point + data with at least one fully-valid reduction slice, the integer coordinate + labels were silently cast to ``float64`` (e.g. ``idxmax(dim="y")`` returned + ``float64`` labels even when ``y`` was ``int64``). The coordinate dtype is now + preserved unless a slice is actually all-``NaN`` and gets filled + (:issue:`7527`). + By `Shurong Cao `_. Documentation ~~~~~~~~~~~~~ diff --git a/xarray/computation/computation.py b/xarray/computation/computation.py index db2125cdd08..3c4a45d120f 100644 --- a/xarray/computation/computation.py +++ b/xarray/computation/computation.py @@ -1015,8 +1015,19 @@ def _calc_idxminmax( res = index._replace(coord[(index.variable,)]).rename(dim) if skipna or (skipna is None and array.dtype.kind in na_dtypes): - # Put the NaN values back in after removing them - res = res.where(~allna, fill_value) + # Put the NaN values back in after removing them. For non-chunked + # inputs we only call `where` when a slice is actually all-NaN: + # calling `where` unconditionally (even for fully-valid slices) silently + # promoted the integer coordinate dtype of the returned labels to the + # float dtype of the default fill value, see GH#7527. For chunked + # (dask) inputs we keep the original unconditional `where`, because + # `allna.any()` would force an extra `compute()` and break dask's + # compute-budget. The dtype promotion is harmless there (the coord is + # already wrapped in a delayed expression). + if is_chunked_array(allna.data): + res = res.where(~allna, fill_value) + elif allna.any(): + res = res.where(~allna, fill_value) # Copy attributes from argmin/argmax, if any res.attrs = index.attrs diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index 9b0a0dcd775..c282193af2c 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -9417,13 +9417,13 @@ def idxmin( int int64 8B 4 float (y) int64 24B 4 0 2 >>> ds.idxmin(dim="x") - Size: 52B + Size: 40B Dimensions: (y: 3) Coordinates: * y (y) int64 24B -1 0 1 Data variables: int >> ds.idxmax(dim="x") - Size: 52B + Size: 40B Dimensions: (y: 3) Coordinates: * y (y) int64 24B -1 0 1 Data variables: int None: + # Regression test for GH#7527: idxmax/idxmin should not promote the + # coordinate dtype of the returned labels when no fill value is needed. + array = xr.DataArray( + [ + [2.0, 1.0, 2.0, 0.0, -2.0], + [-4.0, np.nan, 2.0, np.nan, -2.0], + [np.nan, np.nan, 1.0, np.nan, np.nan], + ], + dims=["y", "x"], + coords={"y": [-1, 0, 1], "x": np.arange(5.0) ** 2}, + ) + # No fully-masked slices, so the integer coordinate dtype is kept. + # The result is named after the reduced dimension and carries its + # (integer) label dtype. + assert array.idxmax(dim="y").dtype == np.int64 + assert array.idxmin(dim="y").dtype == np.int64 + # x is already a float64 coordinate, so reducing over it stays float64 + assert array.idxmin(dim="x").dtype == np.float64 + # Targeted fill only happens for the all-NaN slices, and the result is + # still float there (fill_value default is NaN). + allna = xr.DataArray( + [np.nan, np.nan], + dims=["x"], + coords={"x": np.array([1, 2], dtype=np.int64)}, + ) + assert allna.idxmax().dtype == np.float64 From 608ceb382fffe88db9721934f6990ea6e5265eee Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 26 Aug 2026 08:29:19 +0000 Subject: [PATCH 2/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- xarray/computation/computation.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/xarray/computation/computation.py b/xarray/computation/computation.py index 3c4a45d120f..70fdcd76309 100644 --- a/xarray/computation/computation.py +++ b/xarray/computation/computation.py @@ -1024,9 +1024,7 @@ def _calc_idxminmax( # `allna.any()` would force an extra `compute()` and break dask's # compute-budget. The dtype promotion is harmless there (the coord is # already wrapped in a delayed expression). - if is_chunked_array(allna.data): - res = res.where(~allna, fill_value) - elif allna.any(): + if is_chunked_array(allna.data) or allna.any(): res = res.where(~allna, fill_value) # Copy attributes from argmin/argmax, if any From 1dddd67b9abebb0c214b0b948a720757d4ae5084 Mon Sep 17 00:00:00 2001 From: Deepak Cherian Date: Fri, 28 Aug 2026 08:46:46 -0600 Subject: [PATCH 3/4] Apply suggestion from @dcherian --- xarray/computation/computation.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/xarray/computation/computation.py b/xarray/computation/computation.py index 70fdcd76309..70372cbafb4 100644 --- a/xarray/computation/computation.py +++ b/xarray/computation/computation.py @@ -1015,15 +1015,8 @@ def _calc_idxminmax( res = index._replace(coord[(index.variable,)]).rename(dim) if skipna or (skipna is None and array.dtype.kind in na_dtypes): - # Put the NaN values back in after removing them. For non-chunked - # inputs we only call `where` when a slice is actually all-NaN: - # calling `where` unconditionally (even for fully-valid slices) silently - # promoted the integer coordinate dtype of the returned labels to the - # float dtype of the default fill value, see GH#7527. For chunked - # (dask) inputs we keep the original unconditional `where`, because - # `allna.any()` would force an extra `compute()` and break dask's - # compute-budget. The dtype promotion is harmless there (the coord is - # already wrapped in a delayed expression). + # Put the NaN values back in after removing them. + # We attempt to preserve dtype where we can. if is_chunked_array(allna.data) or allna.any(): res = res.where(~allna, fill_value) From c5f36c9744998d89371e120632c8b7335e9ddc31 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 28 Aug 2026 14:47:13 +0000 Subject: [PATCH 4/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- xarray/computation/computation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xarray/computation/computation.py b/xarray/computation/computation.py index 70372cbafb4..0024511f7c0 100644 --- a/xarray/computation/computation.py +++ b/xarray/computation/computation.py @@ -1015,7 +1015,7 @@ def _calc_idxminmax( res = index._replace(coord[(index.variable,)]).rename(dim) if skipna or (skipna is None and array.dtype.kind in na_dtypes): - # Put the NaN values back in after removing them. + # Put the NaN values back in after removing them. # We attempt to preserve dtype where we can. if is_chunked_array(allna.data) or allna.any(): res = res.where(~allna, fill_value)