-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ENH/API: allow different values for labels/index in DF.reindex __OR__ raise error #21685
Comments
Looks like we should raise in pandas/pandas/util/_validators.py Lines 291 to 292 in dc45fba
ax in out . In this case, out[ax] comes from index= , and we overwrite it with index= .
|
Hello. Is anyone working on this? I would like to take this as my first issue. |
Sorry for the delay @machar94! I don't think anyone is currently working on it. Let us know if you need help getting started. |
Hello @TomAugspurger I know another user wanted to work on the issue but is the issue still open? |
@rcromo I don't see any open PRs addressing this issue. Please feel free to take it. |
@rcromo Yes, please feel free to take it. Unfortunately I wasn't able to follow up on this myself. |
Is this issue still open? |
Still open, and AFAICT no one is actively working on it.
…On Mon, Oct 8, 2018 at 6:15 AM Adam Shamsudeen ***@***.***> wrote:
Is this issue still open?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#21685 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABQHIlKC8QXxKk15x4Sjy-FtwLTPCd-2ks5uizPngaJpZM4U9XIg>
.
|
@TomAugspurger I couldn't replicate this issue as df.duplicated does not support retrun_inverse now? |
@h-vetinari do you know if the original issue has been addressed, and if so which issue fixed it? |
@TomAugspurger @adamshamsudeen The issue is neither outdated nor closed, though I guess I should really separate the proposal that:
The first one is the one that @adamshamsudeen can easily tackle, and that has nothing to do with |
take |
@h-vetinari I found an interesting thing while debugging, the line below never returns the a dict containing a key "label", it always return a dict with key "index" Line 4241 in e1a9b78
Also, the following line removes "labels" from the kwargs effectively making labels argument useless : Line 4245 in e1a9b78
Example:
The output remains the same for any label you throw at it. |
Currently,
DataFrame.reindex
has three overlapping keywords:labels
index
columns
I (naively) expected it to work to pass different values to
labels
/index
(motivating example below), but this does not work. I'm going to make a proposal of how this could be incorporated, but independently from that -- in the current state -- at least an error should be raised on conflicting values tolabels
/index
(or even just using both kwargs).2018-10-08 EDIT: This is as far as necessary for the purpose of raising errors.
Alternatively (or maybe complementarily), one could the following use case for allowing different values for
labels
/index
- as.reindex
(at least by name) has two interpretations:[end of EDIT]
The example is related to what I'm working on in #21645, where I want to construct an inverse to
.duplicated
-- allowing to reconstruct the original object from the deduplicated one.As a toy example:
This is obviously not identical to the original object yet, because -- while we have read the correct indexes from
unique
, we haven't assigned them to the correct output indexes yet.I had been long working with
.loc[]
until v.0.23 started telling me to use.reindex
, and consequently, I wasn't very acquainted with it. I started by trying the following, which would conceptually make sense to me (as opposed to interpreting.reindex(inv)
directly, which would break heaps of code):This was surprising, because
labels
is completely ignored (even though it is the first argument in the call signature), and no warning is raised for swallowing contradictory results.In any case, this is not very high priority, as a more-or-less simple work-around exists, but it is still something to consider, IMO.
The text was updated successfully, but these errors were encountered: