-
-
Notifications
You must be signed in to change notification settings - Fork 18.3k
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.
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
EA interface: add maybe_promote? #24246
Comments
Yeah, I think something like this will eventually be necessary. I don't think it would be part of the user facing API though. Rather, we would call some kind of |
@TomAugspurger Not sure if you were missing a dot in there somewhere, but EDIT: got confused myself about the signature... |
Yep, fixed.
Do you mean ExtensionArray? Or do you think this can be done at the dtype level? I was assuming ExtensionArray, but it'd be great if we could do this without looking at the actual values. |
There are two parameters at play:
This is another (separate) aspect of the whole thing, where
could dispatch to |
I think this is now (or at least in 2.0 will be) unnecessary. Once deprecations are enforced IIRC we'll be able to rewrite maybe_promote as something like:
So the EA authors implement the appropriate behavior in Dtype._get_common_type |
On second thought, I'm coming around on this. The infer_dtype_from+find_common_type pattern leaves something to be desired in a few cases:
|
I'm preparing a PR that addresses the short-comings of
core.dtypes.cast.maybe_promote
uncovered in #23982, and there's currently only a branch in there along the following lines:This does not describe any promotion rules, and (like the rest of
maybe_promote
) is very inconsistent about the returned missing value (see #23982).My suggestion would be to to something like the following:
which would let EA authors decide how to handle dtype promotions (this is relevant e.g. for #24020).
The default could be to always upcast to object, but in many cases, there would be clear conditions for what doesn't upcast (e.g. all integers in the new 'Int' dtypes).
The text was updated successfully, but these errors were encountered: