EA: add back _from_scalar / cast_pointwise_result backwards compat #63367
Conversation
jorisvandenbossche
added
the
ExtensionArray
| original_array: ArrayLike, | ||
| ) -> ArrayLike: | ||
| """ | ||
| Try casting result of a pointwise operation back to the original dtype if |
There was a problem hiding this comment.
is "original dtype" accurate here or would the "dtype family" phrasing be more accurate?
| ---------- | ||
| result : array-like | ||
| Result to cast. | ||
| original_array : array-like |
| values = np.asarray(values, dtype=object) | ||
| return lib.maybe_convert_objects(values, convert_non_numeric=True) | ||
| try: | ||
| return type(self)._from_scalars(values, dtype=self.dtype) |
There was a problem hiding this comment.
IIUC this change is bc older geopandas implements _from_scalars? can we catch+deprecate this so that we can eventually re-simplify this?
There was a problem hiding this comment.
Yes, see my note about that in the top post (first bullet point)
There was a problem hiding this comment.
Right, I'm asking you to catch+deprecate
|
@jbrockmendel do you have a thought about the general idea of the |
Long term i think the extra layer of indirection is not great and I'd prefer the existing pattern. As an interim step for geopandas back-compat I'm fine with it. Instead of exposing maybe_convert_objects can't 3rd party EAs just do |
I didn't do that for back compat (updating the base array implementation of
Personally I think the extra layer of indirection is the cleaner separate of concerns. An EA should not have to deal with general inference rules, just with the implementation of anything related to its own dtype (family). |
I disagree on all counts, but don't care enough to argue about it. |
2 participants
This follows-up on the comments here #62105 (comment) (on the PR that introduced
_cast_pointwise_result/ removed_from_scalars)This PR does a few things:
_cast_pointwise_resultstill does the minimal effort to try to return an instance of itself (the current implementation never returned the EA itself)._from_scalarsfor back compat with external EAs that already implemented this logic by overriding that method. This might only be geopandas, so also fine with eventually deprecating that method (and then_cast_pointwise_resultcan directly call_from_sequence, i.e. what the default impl for_from_scalarsdoes now)cast_pointwise_resultfunctional wrapper aroundEA._cast_pointwise_result. The reason for this is that I don't think the EA should be responsible to know all kinds of (optional) type inference of pandas, it should just take care of the type inference to its own types.result = super()._cast_pointwise_result(values), and we can document that they should do that (and then we can update the base class implementation whenever type inference defaults/options change). My feeling is that it is cleaner to not require that (i.e. let the EA handle just what it knows about, which is less error prone) and have that general logic outside of the EA, but I can also roll that back._cast_pointwise_resultfor EA authors to clarify what this method should do (but this might need to be updated depending on the above)cc @jbrockmendel