Fix padding an integer array with non-finite constant_values (NaN)#11432
Open
PranavAchar01 wants to merge 2 commits into
Open
Fix padding an integer array with non-finite constant_values (NaN)#11432PranavAchar01 wants to merge 2 commits into
PranavAchar01 wants to merge 2 commits into
Conversation
…values Variable.pad only promoted the dtype for the default fill value (dtypes.NA), so padding an integer array with an explicit np.nan raised 'cannot convert float NaN to integer'. Promote the dtype when an explicit non-finite fill value can't be represented by an integer dtype, matching the default-fill behaviour. Finite fills keep numpy's casting behaviour. Closes pydata#6431
There was a problem hiding this comment.
Pull request overview
Fixes DataArray/Dataset/Variable.pad(..., mode="constant", constant_values=np.nan) on integer-backed arrays by promoting the target dtype so non-finite fill values (NaN/Inf) can be represented, instead of raising during padding.
Changes:
- Update
Variable.padto promote integer dtypes when an explicit non-finiteconstant_valuesis provided. - Add regression tests covering explicit
constant_values=np.nanand mixed tuple fills (e.g.(0, np.nan)). - Document the behavior change in
doc/whats-new.rst.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| xarray/core/variable.py | Adds dtype-promotion logic for explicit non-finite constant_values in constant padding mode. |
| xarray/tests/test_dataarray.py | Adds regression assertions for padding integer arrays with explicit NaN fill values. |
| doc/whats-new.rst | Notes the bug fix and dtype-promotion behavior for non-finite explicit constant_values. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1308
to
+1312
| if ( | ||
| mode == "constant" | ||
| and constant_values is not None | ||
| and np.issubdtype(dtype, np.integer) | ||
| ): |
Only inspect plain floating scalars for non-finiteness so unit-aware duck arrays (e.g. pint quantities) passed as constant_values are left untouched, fixing a UnitStrippedWarning in test_pad_unit_constant_value.
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.
Description
Fixes padding an integer array with an explicit non-finite
constant_values(e.g.np.nan), which previously raisedValueError: cannot convert float NaN to integer:Variable.padonly promoted the dtype for the default fill sentinel (dtypes.NA), not for an explicitnp.nan. It now promotes the dtype (vianp.result_type) when an explicit non-finite fill value cannot be represented by an integer dtype, matching the promotion already done for the default fill value. Finite fill values keep numpy's existing casting behaviour (e.g.constant_values=1.23456still truncates on an integer array).Checklist
whats-new.rstAI Disclosure
Tools: Claude Code — prompted to locate and fix issue Bug when padding coordinates with NaNs #6431 with a localized change in
Variable.padplus a regression test. I have reviewed and tested the change locally (thepadtest suites pass).