-
-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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: Perform i8 conversion for datetimelike IntervalTree queries #22988
Conversation
Hello @jschendel! Thanks for submitting the PR.
|
Could you also add tests for |
We currently do not allow an In [2]: pd.IntervalIndex.from_breaks(pd.period_range('2018Q1', freq='Q', periods=3))
---------------------------------------------------------------------------
ValueError: Period dtypes are not supported, use a PeriodIndex instead Interestingly enough, it looks like an In [3]: pd.Interval(pd.Period('2018Q1', freq='Q'), pd.Period('2018Q3', freq='Q'))
Out[3]: Interval(Period('2018Q1', 'Q-DEC'), Period('2018Q3', 'Q-DEC'), closed='right') |
Gotcha. Are there plans for IntervalIndex to support Period? That might get tricky since Periods themselves are conceptually 1 label for an interval of time. |
Codecov Report
@@ Coverage Diff @@
## master #22988 +/- ##
==========================================
+ Coverage 92.18% 92.2% +0.01%
==========================================
Files 169 169
Lines 50823 50865 +42
==========================================
+ Hits 46853 46898 +45
+ Misses 3970 3967 -3
Continue to review full report at Codecov.
|
I'm not aware of any such plans. I don't think the same type algorithms would apply; a There is an open issue to have |
pandas/core/indexes/interval.py
Outdated
@@ -514,6 +519,41 @@ def _maybe_cast_indexed(self, key): | |||
|
|||
return key | |||
|
|||
def _maybe_convert_i8(self, key): | |||
if isinstance(key, Interval): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you document this a bit, and can you simplify at all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Look better?
@mroeschke as indicated above, a |
thanks @jschendel |
git diff upstream/master -u -- "*.py" | flake8 --diff
I've seen a few complaints about this on StackOverflow, so wanted to get this into 0.24.0 before I try implementing larger changes like the new behavior specs and interval accessor.