Drop fails when supplied with an object that implements __getitem__

[max-sixty]

When you supply drop with an item that implements __getitem__, drop doesn't drop it. It does when supplied with the item within a list.

That's because instead of checking is_list_like (here), drop calls _index_labels_to_array which does this. That returns an empty list when supplied with an object that implements __getitem__. (if you want the technicals, that's because list(obj) == [], as __getitem__ used to be the way of implementing iteration. is_list_like ignores that case, I think correctly).

Should _index_labels_to_array instead call is_list_like?

In [182]:

class GetItemObj(object):

    def __init__(self, n):
        self.n = n

    def __getitem__(self, item):
        if item == 'a':
            return self.n
        else:
            raise IndexError

    def __hash__(self):
        return self.n

    def __repr__(self):
        return 'Item: {}'.format(self.n)
In [183]:


index=pd.Index([GetItemObj(i) for i in range(2)])
index
Out[183]:
Index([Item: 0, Item: 1], dtype='object')
In [186]:

index.drop(index[0])
Out[186]:
Index([Item: 0, Item: 1], dtype='object')
In [187]:


index.drop([index[0]])
Out[187]:
Index([Item: 1], dtype='object')

[kawochen]

I'm not sure about putting iterables in an index. Does your proposed method solve the case when your object defines __iter__?

[max-sixty]

@kawochen have a look at is_list_like - it does almost exactly that: https://github.com/pydata/pandas/blob/master/pandas/core/common.py#L2329. The headline of my suggestion is that we consolidate & encapsulate the list checking there, rather than do it differently in different places

Definitely agree putting iterables / list like objects in an index is not a good idea.

[kawochen]

I think I did follow. But I think the support would still be partial. I was thinking

class DictLikeObj(dict):
    def __hash__(self):
        return hash(tuple(sorted(self.items())))

(I am against making it easy to put iterables in Index)

[max-sixty]

Apologies @kawochen, I'm not sure I understand. What's that class for?
My class above was a demonstration, I'm not suggesting that goes anywhere in pandas.
I can do a quick PR - that may makes my point clearer

[kawochen]

@MaximilianR that is a class whose instances I think would still fail to be dropped after your proposed change, so some iterables would work, some wouldn't.

[jreback]

yeh .drop is slightly different, not really sure why. always for cleaning / consolidating. You can try changing and see what breaks (if anything).

And there are slight differences in the iterables & checking and its very very subtle (e.g. the __hash__ DOES matter)