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

API: PeriodIndex.values now return array of Period objects #13988

Closed
wants to merge 1 commit into from

Conversation

sinhrks
Copy link
Member

@sinhrks sinhrks commented Aug 13, 2016

This change itself needs more tests / perf testing, split from the original PR.

@sinhrks sinhrks added Dtype Conversions Unexpected or buggy dtype conversions API Design Period Period data type labels Aug 13, 2016
@sinhrks sinhrks added this to the 0.19.0 milestone Aug 13, 2016
@codecov-io
Copy link

codecov-io commented Aug 13, 2016

Current coverage is 85.26% (diff: 93.25%)

Merging #13988 into master will decrease coverage by <.01%

@@             master     #13988   diff @@
==========================================
  Files           139        139          
  Lines         50446      50475    +29   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43018      43040    +22   
- Misses         7428       7435     +7   
  Partials          0          0          

Powered by Codecov. Last update df2d9ab...d7637c9

@jreback
Copy link
Contributor

jreback commented Aug 14, 2016

this looks fine. pls add a small section for showing the changes in whatsnew (though if this should be merged after #13941 can just add on there (and vice-versa)

@sinhrks
Copy link
Member Author

sinhrks commented Aug 14, 2016

OK, let #13941 be first, because this PR includes some workarounds which can be removed once we have period dtype.

@jreback
Copy link
Contributor

jreback commented Aug 17, 2016

@sinhrks pls rebase. This will be a welcome change!

@jorisvandenbossche
Copy link
Member

Needs a new rebase ;) (for your fixed of the tests)

return self._values.view('i8')

@property
def values(self):
Copy link
Member

Choose a reason for hiding this comment

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

Can you add docstrings for those?

@jorisvandenbossche
Copy link
Member

Can you also add a whatsnew notice?

@sinhrks sinhrks force-pushed the period_values branch 4 times, most recently from 277e9c6 to 90aa8d5 Compare August 21, 2016 04:25
@sinhrks sinhrks changed the title (WIP) API: PeriodIndex.values now return array of Period objects API: PeriodIndex.values now return array of Period objects Aug 21, 2016
@sinhrks
Copy link
Member Author

sinhrks commented Aug 21, 2016

Added whatsnew, now ready for review.

@jreback jreback closed this in e23e6f1 Aug 24, 2016
@jreback
Copy link
Contributor

jreback commented Aug 24, 2016

thanks!

@sinhrks sinhrks deleted the period_values branch August 24, 2016 23:38
.. ipython:: python

pi = pd.PeriodIndex(['2011-01', '2011-02'], freq='M')
pi.values
Copy link
Member

Choose a reason for hiding this comment

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

@sinhrks is it correct to say that to get the integer values as before (if you for some reason were using it), you cas access it through .asi8 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

.asi8 is not really public

.astype is the single method for converting types

Copy link
Contributor

Choose a reason for hiding this comment

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

further the codes are a private impl

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Design Dtype Conversions Unexpected or buggy dtype conversions Period Period data type
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants