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: Add closed to IntervalDtype #19371

Closed
jschendel opened this issue Jan 24, 2018 · 2 comments · Fixed by #38394
Closed

API: Add closed to IntervalDtype #19371

jschendel opened this issue Jan 24, 2018 · 2 comments · Fixed by #38394
Labels
Milestone

Comments

@jschendel
Copy link
Member

jschendel commented Jan 24, 2018

This was brought up by @shoyer in a different issue, and it makes sense to me

Adding closed to IntervalDtype makes sense to me, as in my mind closed is inherently part of the dtype. Since differing closed makes two IntervalIndex incompatible, I'd expect their dtypes to not be equal, but they currently are, which seems a little strange:

In [2]: ii1 = pd.interval_range(0, 3, closed='left')

In [3]: ii2 = pd.interval_range(0, 3, closed='right')

In [4]: ii1.dtype == ii2.dtype
Out[4]: True

There's also a larger discussion as to if adding closed to IntervalDtype would allow us to remove closed as a parameter to the IntervalIndex constructor. My preference would be to keep the closed parameter for user convenience, similar to how CategoricalIndex accepts categories and ordered parameters despite also accepting CategoricalDtype (though maybe this is just for legacy reasons). Willing to be convinced otherwise though.

xref #19263 (comment) (original comment)
xref #19370 (might be rendered moot)

cc @shoyer

@jreback
Copy link
Contributor

jreback commented Jan 24, 2018

+1 on this.

@jbrockmendel
Copy link
Member

+1; recent work in implementing validators this would simplify things

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants