xarray icon indicating copy to clipboard operation
xarray copied to clipboard

Fix: coerce unknown types to O dtype

Open brynpickering opened this issue 6 months ago • 2 comments
trafficstars

Added a check for Python objects that neither have numpy dtype dtype attributes nor are one of the builtin types so that type promotion succeeds and defaults to object dtype.

This deals with cases where user-defined instances are in the array which would otherwise trip up numpy.result_type.

I did think about placing this in xarray.core.dtypes.preprocess_types but chose to place it closest to where it causes issues (namely, on calling xp.result_type).

  • [ ] Closes #9755
  • [ ] Tests added
  • [ ] User visible changes (including notable bug fixes) are documented in whats-new.rst
  • [ ] New functions/methods are listed in api.rst

brynpickering avatar May 20 '25 14:05 brynpickering

do you think we could add a quick comment in the code describing what it's doing, for the less informed of us?

(does anyone know this better?)

max-sixty avatar May 20 '25 18:05 max-sixty

Sure, done.

Another options for this would be to check all the args passed to the compat.array_api_compat.result_type method and if any of them are not a numpy-compatible object or a builtin type then just return np.object_ directly. Passing the args to np.result_type is unnecessary as any object dtype in the list will automatically lead to an object dtype being returned (as it's the lowest common denominator).

brynpickering avatar May 23 '25 13:05 brynpickering

@keewis is there any timeline for this to be reviewed? We're currently stuck with xarray v2024.2.0 as a dependency until this is resolved which is starting to cause issues when used in some working environments.

brynpickering avatar Sep 17 '25 09:09 brynpickering

@keewis @shoyer thanks both. I've committed the suggestion as-is.

brynpickering avatar Nov 05 '25 14:11 brynpickering

Aha @keewis I see we're working on fixing this simultaneously!

With 788eedb the MWE in the originating issue now passes all checks

brynpickering avatar Nov 12 '25 14:11 brynpickering

yeah, I thought I'd help you fix the untested parts of what I suggested

keewis avatar Nov 12 '25 14:11 keewis