Fix quantile raising on an empty result - #11553
Open
chiruu12 wants to merge 6 commits into
Open
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new datetime/timedelta empty-result behavior can conflict with dask gufunc metadata because output_dtypes is still hard-coded to float64, which can break chunked execution.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Fixes quantile on empty results so it returns missing values (rather than raising) and preserves datetime/timedelta dtype semantics, aligning behavior with other reductions and pandas.
Changes:
- Add an empty-result fast path to
Variable.quantile()that validates arguments and returns correctly shaped fill values (NaN/NaT). - Add targeted tests for empty inputs across
Variable,DataArray, andDatasetquantile reductions. - Document the behavior change in the “Bug Fixes” section of the release notes.
File summaries
| File | Description |
|---|---|
| xarray/core/variable.py | Implements the empty-result handling in Variable.quantile() and preserves datetime/timedelta NA semantics. |
| xarray/tests/test_variable.py | Adds comprehensive tests for empty-quantile behavior, argument validation, dtype retention, and extension dtypes. |
| xarray/tests/test_dataarray.py | Adds a DataArray.quantile() regression test for empty inputs. |
| xarray/tests/test_dataset.py | Adds a Dataset.quantile() regression test for empty inputs. |
| doc/whats-new.rst | Notes the bug fix and the new empty-result behavior in release notes. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+2082
to
+2086
| if npa.dtype.kind in "mM": | ||
| fill_value = np.array("NaT", dtype=npa.dtype) | ||
| result_dtype = npa.dtype | ||
| else: | ||
| fill_value = np.nan |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #11549.
_wrapperbuilds the empty result itself instead of moving aqaxis that is not there. Both backing functions are unusable on this input:np.nanquantilereturns the nan but drops the leadingqaxis, somoveaxishandsapply_ufuncthe wrong rank, andnp.quantileraises before returning anything.The trigger is
npa.size == 0, not an empty reduced axis.DataArray(zeros((0, 3))).quantile(0.5, dim="y")reduces overy, which has length 3, and is still empty becausexis.empty.quantile(0.5)AxisErrornanempty.quantile(0.5, skipna=False)IndexErrornanzeros((0,3)).quantile(0.5, dim="y")ValueError(0,)datetime64/timedelta64IndexErrorNaT, dtype keptDatetimes keep their own dtype and NA. That matches the non-empty path,
median, andgroupby_bins(...).quantile()over an empty bin. Returning float64 nan instead madeconcatof an empty and a non-empty result fail to promote.Empty input is validated exactly as non-empty input is, by running the call over one element first so numpy owns the message. Without that,
q=1.5raised on a 5-element array and returned nan on a 0-element one.skipna=Falsereturns nan too, so both paths agree. That differs from numpy, wherenp.quantile([], 0.5)raises, but it is whatmedian(skipna=False)andmean(skipna=False)already do on an empty dimension.Known gap: a pandas extension dtype the non-empty path rejects (
string,category,interval,period,boolean) is still accepted when empty, as it already is forobjectarrays. numpy cannot build a probe array from those dtypes, so onlyqandmethodare checked there.Tests at
Variable,DataArrayandDatasetlevel cover the empty reduction, the empty-but-not-reduced dimension, datetime dtype retention, extension dtypes, and the argument validation. Againstorigin/main, 24 fail and 3 pass; the 3 are guards, two of them against a regression this PR introduced and then fixed.test_variable,test_dataarray,test_dataset,test_groupby,test_weighted,test_computation,test_duck_array_ops,test_units,test_concat: 2961 passed.ruff checkandformatclean.I could not exercise the dask path; dask is not installed in my environment.