ENH: make is_list_like handle non iterable numpy-like arrays correctly
State: closed

- [x] closes #35131
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Enhancement
name: Internals
description: Related to non-user accessible pandas implementation

Comments:
Hello @znicholls! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:



There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2021-02-13 08:43:11 UTC
This is likely to have a non-trivial effect on performance.  It might be time to implement a separate is_list_like (and possibly is_scalar) with less-strict/EA-customizable behavior (cc @jorisvandenbossche I know this has come up w/r/t geopandas)

Can you identify a subset of is_list_like usages where we need the less-strict behavior?
> Can you identify a subset of is_list_like usages where we need the less-strict behavior?

I can check with the other pint-pandas developers. 

Can I also ask which [benchmarks](https://pandas.pydata.org/docs/dev/development/contributing.html#running-the-performance-test-suite) I should look at to examine whether performance has taken a hit?
> I'm not completely sure why, but reverting here for simplicity

yeah, sorry, `getattr` only checks for `AttributeError` but `pint` raises `TypeError`
> yeah, sorry, `getattr` only checks for `AttributeError` but `pint` raises `TypeError`

I think it would have to be `getattr(obj, "ndim", 1) != 0` i.e. you assume everything is iterable unless you have evidence otherwise.
I think the test failures are unrelated to our change?

```
# CI / Web and docs failure text (I think this is the relevant bit)
...
WARNING: autodoc: failed to import method 'sparse.from_spmatrix' from module 'DataFrame'; the following exception was raised:
No module named 'DataFrame'
WARNING: autodoc: failed to import method 'sparse.to_coo' from module 'DataFrame'; the following exception was raised:
No module named 'DataFrame'
WARNING: autodoc: failed to import method 'sparse.to_dense' from module 'DataFrame'; the following exception was raised:
No module named 'DataFrame'
WARNING: autodoc: failed to import method 'sparse.from_coo' from module 'Series'; the following exception was raised:
No module named 'Series'
WARNING: autodoc: failed to import method 'sparse.to_coo' from module 'Series'; the following exception was raised:
No module named 'Series'
...
```
> This is likely to have a non-trivial effect on performance. 

To have an idea about this, I timed the following cases:

```
a1 = [1, 2, 3]
a2 = np.array([1, 2, 3])
a3 = np.array([1, 2, 3])[0] 
```

```
In [2]: %timeit pd.api.types.is_list_like(a1)      
390 ns ± 1.56 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)   # master  
553 ns ± 1.77 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)   # PR

In [3]: %timeit pd.api.types.is_list_like(a2) 
405 ns ± 0.972 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)   # master
408 ns ± 0.745 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)   # PR

In [4]: %timeit pd.api.types.is_list_like(a3)  
395 ns ± 1.6 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)   # master
411 ns ± 1.55 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)    PR
```

So for numpy arrays the slowdown is only very limited (I assume because in this case we needed to look up `obj.ndim` anyway, so then then `getattr(obj, "ndim")` seems to be similar). For actual lists, the slowdown is more significant, though (but I would think the array cases are most relevant for performance).


> It might be time to implement a separate is_list_like (and possibly is_scalar) with less-strict/EA-customizable behavior

It's not only EAs, though, but eg also for object dtype (see https://github.com/pandas-dev/pandas/issues/26333, https://github.com/pandas-dev/pandas/issues/27911)

But indeed, a less strict `is_list_like` check depending on the dtype of the involved data might be worth exploring, although not that easy I think. The first thing would be to identify the cases where this less strict case is actually important. I suppose the problems in pint-pandas mainly occur in indexing + arithmetic/comparison operations ?
These were the tests which started passing when deleting the `__iter__` method from the scalar class to make is_list_like return False:

<details>

- TestGroupby::test_groupby_apply_identity
- TestMethods::test_fillna_copy_frame
- TestMethods::test_fillna_copy_series
- TestMissing::test_fillna_scalar
- TestMissing::test_fillna_series
- TestMissing::test_fillna_frame
- TestReshaping::test_unstack[series-index1]
- TestReshaping::test_unstack[series-index3]
- TestReshaping::test_unstack[frame-index1]
- TestReshaping::test_unstack[frame-index3]
- TestSetitem::test_setitem_mask_broadcast[loc]
- TestSetitem::test_setitem_sequence_broadcasts[True]
- TestSetitem::test_setitem_integer_array[True-list]
- TestSetitem::test_setitem_integer_array[True-integer-array]
- TestSetitem::test_setitem_integer_array[True-numpy-array]
- TestSetitem::test_setitem_slice[True]
- TestSetitem::test_setitem_loc_iloc_slice
</details>

It looks like using numpy.iterable speeds up the list and array cases but not the numpy scalar one (which I think makes sense as you have to do some more thinking on that one). (This comment is pending tests passing...)

```python
import numpy as np
import pandas as pd

a1 = [1, 2, 3]
a2 = np.array([1, 2, 3])
a3 = np.array([1, 2, 3])[0]


%timeit pd.api.types.is_list_like(a1)
289 ns ± 16.6 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each) # PR
447 ns ± 7.82 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each) # master

%timeit pd.api.types.is_list_like(a2)
302 ns ± 10.2 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each) # PR
466 ns ± 4.96 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each) # master

%timeit pd.api.types.is_list_like(a3)
596 ns ± 2.6 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each) # PR
440 ns ± 1.68 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each) # master
```
> Can you identify a subset of is_list_like usages where we need the less-strict behavior?

Having dug a bit deeper, we're going to need to back to the drawing board on this. My impression is that in order to be handled by `is_list_like`, `is_scalar` and `item_from_zerodim` correctly, the class has to basically be a numpy array (or sub-class thereof). I'll go back to the pint-pandas devs and we can discuss how we go forward there, including identifying the cases where a less strict `is_list_like`, `is_scalar` and `item_from_zerodim` would solve our problems. 

I'm happy to close this in the meantime. However it would be great if we could reopen #22582 to at least add pint-pandas to the docs (buggy as it is).
I would be nice to talk to geopandas devs to see if we can join forces. We might be facing the same problem.
@hgrecco you are alread talking to them ;) (well, at least, I am one of the geopandas devs) 
See also my comment at https://github.com/pandas-dev/pandas/issues/35131#issuecomment-654053344: although very much related, I think it's not exactly the same. As for GeoPandas, the scalar elements we put in a Series are actually iterable (in contrast to the (duck-typed) numpy scalars, which are iterable in theory (have the dunder method), but raise once you try to iterate). 
> It looks like using numpy.iterable speeds up the list and array cases

It seems that `np.iterable` is actually just a try/except of `iter(value)`:

```
    try:
        iter(y)
    except TypeError:
        return False
    return True
```

(if we decide to use this, I would prefer doing the code above explicitly instead of using `np.iterable`, as I think most people are not familiar with that numpy method, and the code is simple).

But so basically it is a choosing between `isinstance(value, Iterable)` + special case check to exclude `ndim==0` vs try/except of `iter(value)` (not fully sure if there are other consequences of the choice between both)
@jorisvandenbossche thanks for the clarification. 

@znicholls Have you tried reinstating `getattr(obj, "ndim", 1) != 0` and moving it first? Python's `and/or` are shortcircuit operators so we might want to put first those checks which are fast and likely to return false.
Reverting the changes as suggested by @hgrecco leads to a slowdown for lists, no change for numpy arrays and a speedup for numpy scalars (essentially the reverse of previously)

```python
import numpy as np
import pandas as pd

a1 = [1, 2, 3]
a2 = np.array([1, 2, 3])
a3 = np.array([1, 2, 3])[0]


%timeit pd.api.types.is_list_like(a1)
640 ns ± 6.82 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each) # PR
447 ns ± 7.82 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each) # master

%timeit pd.api.types.is_list_like(a2)
414 ns ± 17.1 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each) # PR
466 ns ± 4.96 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each) # master

%timeit pd.api.types.is_list_like(a3)
385 ns ± 10.7 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each) #PR
440 ns ± 1.68 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each) # master
```
We still have the `is_scalar` and `item_from_zerodim` issues of course...
@jbrockmendel I went through and identified where different `is_list_like` and `is_scalar` would be needed, see https://github.com/pandas-dev/pandas/compare/master...znicholls:handle-extension-arrays. Is that helpful?
@jorisvandenbossche and @jbrockmendel do you have a sense of how best to proceed here? My impression is that this will require two, if not three, steps:

1. decide where we would like to use less strict implementations of `is_list_like` and `is_scalar` (see https://github.com/pandas-dev/pandas/compare/master...znicholls:handle-extension-arrays for where changes are needed for our Pint extension array implementation to work)
1. implement less strict `is_list_like` and `is_scalar` in the internals where there is no performance issue
1. (with luck, not a necessary step) come up with solutions for places where we'd like to have less strict implementations but the performance penalties are an issue
> do you have a sense of how best to proceed here? My impression is that this will require two, if not three, steps:

These steps look about right.  This likely will take a while.
Ok should I make a PR based on https://github.com/pandas-dev/pandas/compare/master...znicholls:handle-extension-arrays as a start?
what is the status here? @znicholls 
> what is the status here?

Having rebased onto master, the issues raised here still exist (the new tests still fail).

Re-reading the issues and related PRs, I think https://github.com/pandas-dev/pandas/pull/35127#issuecomment-657391697 is the best summary i.e. the first step is identifying where we need the changes, the second step is implementing the changes (with my suggestion being less strict `is_list_like` and `is_scalar` to avoid performance hits but this might not be the best path forward once we've done step 1), the third step is finalisation (more or less).

I've had a go at doing step 1 (see https://github.com/pandas-dev/pandas/compare/master...znicholls:handle-extension-arrays). I'm waiting for advice on:

1. whether those identified spots to change make sense (alterations to `is_scalar` might also be needed but I'm struggling to work out why so have just left them out for now) 
1. whether going down the route of adding new implementations makes sense or whether there is a better route




@znicholls ok i see your comments above, not sure where this is going. I am not sure what your proposed implementation even looks like.
The backstory is #35131. Hopefully that explains why we are looking to make such a change? (@jorisvandenbossche just double checking no progress has been made elsewhere that I've missed?)

A concern about performance impacts was raised in this comment https://github.com/pandas-dev/pandas/pull/35127#issuecomment-653954875. After some discussion, we arrived at https://github.com/pandas-dev/pandas/pull/35127#issuecomment-657391697. 

For completeness, we also did some checks about performance penalties, e.g. https://github.com/pandas-dev/pandas/pull/35127#issuecomment-654069720, https://github.com/pandas-dev/pandas/pull/35127#issuecomment-654530394 and https://github.com/pandas-dev/pandas/pull/35127#issuecomment-655178794. None of our solutions was perfect.

> I am not sure what your proposed implementation even looks like.

That makes sense, because I am also unsure. I've only got to step 1 of https://github.com/pandas-dev/pandas/pull/35127#issuecomment-657391697. Based on the 'hot spots' identified in https://github.com/pandas-dev/pandas/compare/master...znicholls:handle-extension-arrays, I am looking for advice about whether we could use less strict (potentially slower) implementations of `is_list_like` in these places.

If we can use less strict implementations, my only proposed implementation is the changes made in this PR i.e. https://github.com/pandas-dev/pandas/compare/master...znicholls:non-iterable-duck-arrays. In short, the less strict implementation would check for objects which are numpy-style zero-dimensional arrays with `not (hasattr(obj, "ndim") and obj.ndim == 0)` rather than `not (util.is_array(obj) and obj.ndim == 0)` because the latter only works for numpy arrays and doesn't work for **numpy-style** arrays (such as Pint). 
@znicholls what is the status here
> what is the status here

I would greatly appreciate your (or another pandas developer) thoughts on https://github.com/pandas-dev/pandas/pull/35127#issuecomment-757555672. I'm not sure I can sum it up any more succinctly than what is in that comment.
> If we can use less strict implementations, my only proposed implementation is the changes made in this PR i.e. master...znicholls:non-iterable-duck-arrays. In short, the less strict implementation would check for objects which are numpy-style zero-dimensional arrays with not (hasattr(obj, "ndim") and obj.ndim == 0) rather than not (util.is_array(obj) and obj.ndim == 0) because the latter only works for numpy arrays and doesn't work for numpy-style arrays (such as Pint).

i think this is reasonable. of course asv's will tell us whether its an acceptable perf hit. trying to balance additional complexity (seems minor in this case) with perf vs enhancements.
> i think this is reasonable. of course asv's will tell us whether its an acceptable perf hit. trying to balance additional complexity (seems minor in this case) with perf vs enhancements.

Ok great thank you, I will start on that path then.
@jreback I have tried to start small with #39790, if you could take a look that would be great