ExtensionArray constructor issue

[TomAugspurger]

I don't think that pandas should make any assumptions on how subclasses __init__ method. Currently we assume that the first positional argument accepts an instance of the class or a sequence of the class's scalar type.

I'd rather break that into two distinct methods.

@classmedthod
def from_extension_array(cls, extension_array: ExtensionArray) -> ExtensionArray:
    """Construct a new ExtensionArray from an instance of the same type

    Parameters
    ----------
    extension_array : ExtensionArray
        An instance of 'cls'

    Returns
    -------
    ExtensionArray
    """


ScalarType = ExtensionArray.dtype.type

@classmethod
def from_scalars(cls, scalars: Sequence[ScalarType]) -> ExtensionArray:
    """Construct a new ExtensionArray from a sequence of the scalar type

    Parameters
    ----------
    scalars : Sequence[ScalarType]
        A sequence of cls.dtype.type, the scalar type for this array

    Returns
    -------
    ExtensionArray
    """

I think these should be abstract. For many subclasses, a simple cls(arg) should suffice.

[jreback]

what is the reason for this?

[TomAugspurger]

In case a subclass has a different __init__ signature.

[jorisvandenbossche]

+1

I only would make them private _from_scalars and _from_extension_array, to give the extension author to give them more meaningful public names if they want to add something like that (eg in geopandas we currently call 'from_scalars' from_shapely).

Also we could here make clear expectations about data being copied or not (and for that, maybe add a copy keyword.

[TomAugspurger]

Also we could here make clear expectations about data being copied or not

Yes, I thought about making that part of the _from_extension_array one, but I wasn't sure if all subclasses would be able to honor it, and what implications that would have for pandas using it, since we couldn't really rely on it. But if it's just a performance thing, subclasses maybe having to copy, even with copy=Falsse, isn't such a big deal.

[jorisvandenbossche]

I think the fact we cannot rely upon is if it always can be done without copy, but we can rely on requiring a copy? (similar to the copy keyword in np.array)

[TomAugspurger]

similar to the copy keyword in np.array

Yes, that sounds right.

Also +1 to making them private

Are there any other cases we should consider? In cyberpandas I have two others

• _from_ndarray: https://github.com/ContinuumIO/cyberpandas/blob/e9e8f195601e188e58e3d6a2038f7e0e28c1737e/cyberpandas/ip_array.py#L220 That's a zero-copy constructor from a NumPy structured array with the correct fields. Too specific for pandas
• from_bytes: from a buffer of bytes. Way too specific to IP Addresses.

[jreback]

and we already have and use _construtor in pandas proper. Having alternative constructors is fine, except we have no way to actively select when / how one is called. So this is a huge break with current pracitce. I am aa huge -1 on doing this w/o actually changing pandas to do this. I don't think this is possible.

[TomAugspurger]

This is as much (or more so) for external code as it is for pandas. We can't impose the same restrictions on 3rd party libraries as we do internally.

The use of these constructors will be limited. The only place returning ExtensionArrays will be in ExtensionArray itself. It's not something that's going to pollute all of our internals.

[jorisvandenbossche]

We are adding functionality here. There is no place in pandas where we currently call this for ExtensionArrays, so each call we add we can evaluate to see which one we should use.

For example, Categorical (currenlty the only ExtensionArray in pandas) has a _constructor, but it is used in a very specific way (it is always used with passing codes (with fastpath=True)), so that is not something we could rely upon in general anyhow. I think it would be clearer if we had an internal _from_codes or _constructor_from_codes (with less checks than the public from_codes) instead of a _constructor(..., fastpath=True)

[jorisvandenbossche]

The use of these constructors will be limited. The only place returning ExtensionArrays will be in ExtensionArray itself. It's not something that's going to pollute all of our internals.

Indeed, eg the _from_scalars would currently mainly be used in the tests (apart from the 'fallback' unique implementation)

[shoyer]

I agree with this change.

[jorisvandenbossche]

Do we actually currently have cases where we would use _from_extension_array? Because if we already know it is an extension array, why passing it through a constructor instead of just returning it or taking a copy of it directly?

[TomAugspurger]

Do we actually currently have cases where we would use _from_extension_array?

Good point. I initially included it because that's a case I ran into with cyberpandas' IPArray.__init__. But indeed, we don't need it on pandas side since we can just return the EA (copying if needed).