Skip to content

Fix nightly test failures #11204

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.

Sign up for GitHub

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

Merged
jsignell merged 3 commits into from
Mar 10, 2026
+3 −4
Merged

Fix nightly test failures #11204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d73b96e
Filter deprecated warning
jsignell Feb 25, 2026
2b63c03
Remove inspect.signature raises error test. The example no longer rai…
jsignell Feb 25, 2026
9ebf67d
Type of error changed in numpy 2.4
jsignell Feb 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions xarray/core/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,15 +205,15 @@ def get_valid_numpy_dtype(array: np.ndarray | pd.Index) -> np.dtype:


def maybe_coerce_to_str(index, original_coords):
"""maybe coerce a pandas Index back to a nunpy array of type str
"""maybe coerce a pandas Index back to a numpy array of type str

pd.Index uses object-dtype to store str - try to avoid this for coords
"""
from xarray.core import dtypes

try:
result_type = dtypes.result_type(*original_coords)
except TypeError:
except (TypeError, ValueError):
pass
Comment on lines 214 to 217
Copy link
Collaborator

@keewis keewis Feb 26, 2026

Choose a reason for hiding this comment

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

It looks like this was only supposed to cast object dtype arrays to numpy string, in which case this is fine.

If we actually do want to cast the pandas string dtype to a numpy string dtype, however, we may need to change that (@dcherian, do you remember what our policy on pandas dtypes was?)

Copy link
Member Author

@jsignell jsignell Feb 27, 2026

Choose a reason for hiding this comment

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

Right yeah there might be something else that needs doing to support these more completely. For instance you can't write them to netcdf or zarr, but I tested with older versions of pandas/numpy and it looks like this test has always hit this except it's just the type changed from TypeError to ValueError

Copy link
Contributor

@dcherian dcherian Mar 10, 2026

Choose a reason for hiding this comment

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

If we actually do want to cast the pandas string dtype to a numpy string dtype,

I think at the moment we do; and then relax it intentionally. However, we seem to have done pretty badly at that haha. Things seem to keep breaking.

Copy link
Member Author

@jsignell jsignell Mar 10, 2026

Choose a reason for hiding this comment

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

Ok so something to keep working towards, but for the purposes of getting CI green again, maybe no worse than it was before is fine for now?

Copy link
Contributor

@dcherian dcherian Mar 10, 2026

Choose a reason for hiding this comment

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

yes absolutely

else:
if result_type.kind in "SU":
Expand Down
1 change: 1 addition & 0 deletions xarray/tests/test_computation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2646,6 +2646,7 @@ def test_complex_number_reduce(compute_backend):
da.min()


@pytest.mark.filterwarnings("ignore:numpy.fix is deprecated.")
def test_fix() -> None:
val = 3.0
val_fixed = np.fix(val)
Comment on lines +2649 to 2652
Copy link
Collaborator

@keewis keewis Feb 26, 2026

Choose a reason for hiding this comment

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

we could also just remove (or deprecate?) support for fix, there's a clear alternative in np.trunc that does the same and is more efficient

Copy link
Member Author

@jsignell jsignell Feb 27, 2026

Choose a reason for hiding this comment

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

Yeah sorry I meant to put notes on each of these. I looked at the PR where this was added and it was specifically resolving an infinite recursion that was possible within apply_ufunc for certain kwargs. I just checked and you don't get the same recursion issue with np.trunc. So I think the alternative is to delete the test.

Expand Down
2 changes: 0 additions & 2 deletions xarray/tests/test_dataarray.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4908,8 +4908,6 @@ def exp_decay(t, n0, tau=1):
param_names = ["a"]
params, func_args = _get_func_args(np.power, param_names)
assert params == param_names
with pytest.raises(ValueError):
_get_func_args(np.power, [])
Comment on lines -4911 to -4912
Copy link
Collaborator

@keewis keewis Feb 26, 2026

Choose a reason for hiding this comment

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

do you know what about numpy.power changed? I can't find a PR / changelog entry other than one that changes its behavior on a old CPU architecture (AIX)

Copy link
Member Author

@jsignell jsignell Feb 27, 2026

Choose a reason for hiding this comment

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

I didn't find any information specifically on np.power but from reading the comments on the original PR it seemed like sometimes function parameters weren't discoverable by inspect.signature my understanding is that this had to do with how ufuncs are constructed in numpy or possibly it has to do with C based functions. I just figured numpy might have got a better system for passing parameter information around.

Copy link
Contributor

@dcherian dcherian Mar 10, 2026

Choose a reason for hiding this comment

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

I just figured numpy might have got a better system for passing parameter information around

yeah i think ufuncs have better python-accessible signatures now


@requires_scipy
@pytest.mark.parametrize("use_dask", [True, False])
Expand Down
Loading