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: unstack fails in PeriodIndex #7041

Merged
merged 1 commit into from
May 10, 2014
Merged

Conversation

sinhrks
Copy link
Member

@sinhrks sinhrks commented May 5, 2014

Closes #4342.

Also changed Categorical and PeriodIndex.factorize to make index sorted like other indexes do.

@@ -458,6 +458,7 @@ Bug Fixes
- Bug in timeseries-with-frequency plot cursor display (:issue:`5453`)
- Bug surfaced in groupby.plot when using a ``Float64Index`` (:issue:`7025`)
- Stopped tests from failing if options data isn't able to be downloaded from Yahoo (:issue:`7034`)
- Bug in ``unsatck`` raises ``ValueError`` when ``MultiIndex`` contains ``PeriodIndex`` (:issue:`4342`)
Copy link
Contributor

Choose a reason for hiding this comment

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

unstack

@jreback
Copy link
Contributor

jreback commented May 5, 2014

did anything change because of the sorting change?

@jreback jreback added this to the 0.14.0 milestone May 5, 2014
@jreback
Copy link
Contributor

jreback commented May 5, 2014

@sinhrks fyi, in general ping me when you think something is ready for merging (esp if its marked for 0.14). try to make green! if its not, ok to ping as well and will take a look

@sinhrks
Copy link
Member Author

sinhrks commented May 5, 2014

Sorting change only affects to Categorical creation with PeriodIndex. Because:

  • Categorical calls Index.factorize if it has the method. Currently, PeriodIndex is the only class which has the method. Thus, any other classes and indexes are not affected.
  • PeriodIndex.factorize use sort=False as default (which derived from core.algorithms.factorize). Thus, any other function which use PeriodIndex.factorize will get the same result as before.

@jreback
Copy link
Contributor

jreback commented May 5, 2014

ok; looking at this code, I think you can define factorize for Index/Series in core/base.py,

then just do the test hasattr(labels,'factorize')

@jreback
Copy link
Contributor

jreback commented May 10, 2014

ok, i think you can rebase this and then good?

@jreback
Copy link
Contributor

jreback commented May 10, 2014

or close after #7090?

@sinhrks
Copy link
Member Author

sinhrks commented May 10, 2014

Should be rebased, I will finish it soon.

@sinhrks
Copy link
Member Author

sinhrks commented May 10, 2014

@jreback Rebased, and get green.

jreback added a commit that referenced this pull request May 10, 2014
BUG: unstack fails in PeriodIndex
@jreback jreback merged commit 49f3616 into pandas-dev:master May 10, 2014
@jreback
Copy link
Contributor

jreback commented May 10, 2014

thanks!

@sinhrks sinhrks deleted the unstack branch May 10, 2014 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Period Period data type Reshaping Concat, Merge/Join, Stack/Unstack, Explode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Series with MultiIndex with PeriodIndex can't do unstack()
2 participants