Skip to content

ENH: Have Period/PeriodIndex inherit from Interval/IntervalIndex #19419

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

Closed
jschendel opened this issue Jan 27, 2018 · 5 comments
Closed

ENH: Have Period/PeriodIndex inherit from Interval/IntervalIndex #19419

jschendel opened this issue Jan 27, 2018 · 5 comments
Labels
API Design Enhancement Interval Interval data type Period Period data type

Comments

@jschendel
Copy link
Member

jschendel commented Jan 27, 2018

Since Period objects are inherently intervals of time, it seems like there should be a way to access Interval properties and methods. Inheritance has been suggested in passing in other issues, and seems reasonable.

As an example benefit, #9089 suggests implementing Period.duration. If Period were to inherit from Interval, this would already be implemented via the Interval.length property:

In [2]: p = pd.Period('2018Q1', freq='Q')

In [3]: p_iv = pd.Interval(p.start_time, (p + 1).start_time, closed='left')

In [4]: str(p_iv)
Out[4]: '[2018-01-01, 2018-04-01)'

In [5]: p_iv.length
Out[5]: Timedelta('90 days 00:00:00')

Likewise for PeriodIndex inheriting from IntervalIndex. Would be nice to have cross compatibility between the two, e.g. IntervalIndex.overlaps(Period) or PeriodIndex.get_loc(Interval).

@jreback
Copy link
Contributor

jreback commented Jan 27, 2018

there is an existing issue about making Period inherit from Interval (and same with PI and II)

it might not be bad

@jreback jreback added Period Period data type Interval Interval data type Enhancement API Design labels Jan 28, 2018
@jschendel
Copy link
Member Author

@jreback : I recall seeing that mentioned somewhere, but can't find a corresponding issue. Do you happen to know where it is?

Also, would adding this method still be worthwhile if inheritance is implemented? Maybe if someone wanted to add a non-compatible Interval to a PeriodIndex, they'd need convert to IntervalIndex first? Not sure how common/useful that would be. Might just close this if there's no clear benefit to having this in addition to inheritance.

@jreback
Copy link
Contributor

jreback commented Jan 30, 2018

i couldn’t find the issue but iirc some disscussiom on the II original issue maybe

yeah inheritance would be nice here

can repurpose this issue

@jschendel jschendel changed the title ENH: Add Period/PeriodIndex.to_interval() ENH: Have Period/PeriodIndex inherit from Interval/IntervalIndex Jan 30, 2018
@jbrockmendel
Copy link
Member

After some thought, I'm -1 on this. Assuming the Interval in question would have endpoints that are themselves Timestamps, this would limit the range of Period/PeriodIndex to the same bounds as Timestamp. Representing distant-past/future times is a use case that right now can only be done with Periods.

@jbrockmendel
Copy link
Member

Closing based on the argument above. Feel free to re-open for further discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Design Enhancement Interval Interval data type Period Period data type
Projects
None yet
Development

No branches or pull requests

3 participants