BUG/API: maybe drop recently introduced "takeable" from reindex?

[immerrr]

I think there might be an issue with reindex(..., takeable=True) introduced in a recent release and you might want to consider deprecating/dropping it while it hasn't crept to user code too much.

The main reason is that reindex takeable is conceptually different from reindex: the latter can introduce new rows with values that must be filled while the former cannot. Essentially, reindex takeable is no more than a fancy indexing operation of numpy which is already available via iloc indexer. If you consider reindex parameters, you can see there' no additional value in reindex takeable:

• method, limit & fill_value parameters are useless since no empty cells can appear
• level parameter should be available to user via fancy multiindex slicing added recently
• copy is also achievable via .copy() method

So not only it adds no value, but it also increases complexity:

• externally, by adding yet another way to do fancy indexing
• internally, by adding more code paths to follow when implementing & executing reindex

The last point has already caused a bug:

In [1]: s = pd.Series(list('abc'), index=np.arange(3)[::-1])

In [2]: s
Out[2]: 
2    a
1    b
0    c
dtype: object

In [3]: s.take([2,1,0])
Out[3]: 
0    c
1    b
2    a
dtype: object

In [4]: s.reindex([2,1,0], takeable=True)
Out[4]: 
2    a
1    b
0    c
dtype: object

In [5]: s.iloc[[2,1,0]]
Out[5]: 
2    a
1    b
0    c
dtype: object

Of course, this can be fixed by inserting workarounds here and here but this will increase complexity even more and won't address the conceptual issue.

[jreback]

if u can get tests to pass without it go for it

it is not a user option at all but triggered via an iloc call
that takes the same path

I don't disagree with any of your points and maybe it can be removed
however how would I don't see an easy way around differentiating between passing thru an indexer that is positional vs a label reindexer

this wasn't that recent

[immerrr]

it is not a user option at all but triggered via an iloc call

You did see me do s.reindex([2,1,0], takeable=True), right? :)

Also, I'm pretty sure I've seen a description of takeable kwarg in API reference this morning before posting.

[jreback]

its not meant to be called by the user
and making another function call was just silly

just try to take it out

I am sure it's possible but not trivial

maybe you can find a nice way of refactoring
I just didn't have time to dive into it

[immerrr]

its not meant to be called by the user

IDK, I use reindex to align containers all the time.

[jreback]

your example looks fine to me
what is the bug?
it's doing exactly what it says
as a user you should use 'take' not this of you want to reindex by position

go ahead take it out and see what happens

if you can make things work without it great

[immerrr]

your example looks fine to me
what is the bug?

The series has an index of integers counting down. take is advertised to operate on locations, not labels, hence when I do take([2,1,0]), the series is reversed and this is expected.

Now, reindex(..., takeable=True) is advertised to

takeable : boolean, default False
treat the passed as positional values

hence, I'd expect it to return the same reversed series as take did but in this particular case it does not.

This happens because there are (multiple) optimization shortcuts that check if reindex argument matches the actual index and if the reindexing can be avoided altogether. The problem is that those shortcuts don't consider takeable and thus may try to match labels with positions which happens in this case. With the values I've provided the check (wrongly) succeeds and returns the series as-is.

[jreback]

the point is takeble is an internal call
so it doesn't apply

and has very specific uses/guarantees

you can change it to _takeable if u want or try to remove it from the public interface

the problem is that reindex is called everywhere and so adding another level of call is possible but a big. change

I agree that this was a work around
you were welcome to try to fix it

[immerrr]

Ok, I'll see to it.