Skip to content
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

BUG: Fixes KeyError when indexes don't overlap. #12287

Closed
wants to merge 1 commit into from
Closed

BUG: Fixes KeyError when indexes don't overlap. #12287

wants to merge 1 commit into from

Conversation

tshauck
Copy link
Contributor

@tshauck tshauck commented Feb 11, 2016

Closes #10291

@TomAugspurger
Copy link
Contributor

Can you edit your first post to include "Closes #10291, Closes #12133" so that Github will auto-close those issues when we merge?

Also would need a release note in doc/source/whatsnew/v0.18.0 under bug fixes and maybe add a note to the docstring of crosstab explaining that this is the outcome for non-overlapping indices. Thanks!

@TomAugspurger TomAugspurger added the Reshaping Concat, Merge/Join, Stack/Unstack, Explode label Feb 11, 2016
@TomAugspurger TomAugspurger added this to the 0.18.0 milestone Feb 11, 2016
s2 = pd.Series([4, 5, 6], index=[4, 5, 6])

actual = crosstab(s1, s2)
self.assertTrue(actual.empty)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

construct the actual result and use assert_frame_equal

@jreback
Copy link
Contributor

jreback commented Feb 11, 2016

you need a test to cover #12133

@jreback
Copy link
Contributor

jreback commented Feb 11, 2016

pls add a whatsnew as well

@jreback jreback added the Bug label Feb 11, 2016
@tshauck
Copy link
Contributor Author

tshauck commented Feb 11, 2016

Thought it may be easier in the near term to reduce the scope of this change to fixing the issue associated with #10291 and not attempt a fix at #12133.

I've added #12133 to my todo list so I'll try to get back to it. Hopefully it's as simple as adding a test case.

@jreback jreback closed this in dcc7cca Feb 12, 2016
@jreback
Copy link
Contributor

jreback commented Feb 12, 2016

thanks @tshauck

@tshauck tshauck deleted the fix-non-overlapping-crosstab branch February 12, 2016 05:29
@Winand
Copy link
Contributor

Winand commented Feb 15, 2016

if values_passed and not values_multi and not table.empty:
Excuse me, but how does this cover #12133? index names are dropped earlier in if not dropna: block. We need to pass names to from_arrays:

m = MultiIndex.from_arrays(cartesian_product(table.index.levels), names=table.index.names)
...
m = MultiIndex.from_arrays(cartesian_product(table.columns.levels), names=table.columns.names)

UPD: sorry, i thought that it's as simple as adding a test case means that it covers #12133 but tests are needed
UPD2: #12327

@jreback
Copy link
Contributor

jreback commented Feb 15, 2016

if u read the comments about it does not cover 12133

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Reshaping Concat, Merge/Join, Stack/Unstack, Explode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants