ExtensionArray.__fillna__ should not use lib.is_scalar

[TomAugspurger]

lib.is_scalar is a bit too strict for the check we do in

pandas/pandas/core/arrays/base.py

Line 272 in 7273ea0

if not is_scalar(value):

. For example, it doesn't condisder ipaddress.IPv4Address as a scalar.

In [1]: from pandas._libs import lib

In [2]: import ipaddress

In [3]: lib.is_scalar(ipaddress.IPv4Address(1))
Out[3]: False

Replacing that check with a hasattr(value, '__len__') should be sufficient.

[jorisvandenbossche]

The same for geopandas.

However, in general, also the len check might not always be good, because eg for geopandas a MultiLineString has a length .. . Or for the json-array, a dict also has a length.

This might not be easy to solve in general, it's mainly due to our flexible API where we need to interpret a lot about what is passed in ..

[jorisvandenbossche]

Actually, even simpler example where a length check will not work: strings.
(but of course, for only this one it would be easy to add to an additional check)

[TomAugspurger]

Hmm hadn't thought that through.

Perhaps we require that ExtensionArray.fillna(array) requires array to be an instance of the ExtensionArray? That should remove all ambiguity, at the cost of requiring IPArray([]).fillna(['192.168.1.1']) to be written as IPArray([]).fillna(IPArray(['192.168.1.1'])).

[jorisvandenbossche]

at the cost of requiring IPArray([]).fillna(['192.168.1.1']) to be written as IPArray([]).fillna(IPArray(['192.168.1.1'])).

But in this case you can still fill with IPArray([]).fillna('192.168.1.1') (filling with a scalar) ?

But such a requirement might be a good idea. Or we could also check it being "array-like" (check if is has a shape and if that shape matches).
In the current implementation, I don't think that a list (instead of array) would work anyhow, because we apply a boolean mask on it?

[TomAugspurger]

is_array_like seems to be what we want, since we do use boolean masking on it.

…

On Mon, Mar 19, 2018 at 12:13 PM, Joris Van den Bossche < ***@***.***> wrote: at the cost of requiring IPArray([]).fillna(['192.168.1.1']) to be written as IPArray([]).fillna(IPArray(['192.168.1.1'])). But in this case you can still fill with IPArray([]).fillna('192.168.1.1') (filling with a scalar) ? But such a requirement might be a good idea. Or we could also check it being "array-like" (check if is has a shape and if that shape matches). In the current implementation, I don't think that a list (instead of array) would work anyhow, because we apply a boolean mask on it? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#20411 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQHIrUbFUoz-ormBr7bN5A8PzEJNsdVks5tf-c3gaJpZM4SwWBZ> .