WIP: overlaps handling of empty intervals in Interval and IntervalArray
#26937
Conversation
ghost
mentioned this pull request
ghost
changed the title
Interval.overlaps handling of empty intervalsInterval.overlaps handling of empty intervals
Codecov Report
@@ Coverage Diff @@
## master #26937 +/- ##
===========================================
- Coverage 91.87% 41.1% -50.77%
===========================================
Files 180 180
Lines 50743 50747 +4
===========================================
- Hits 46620 20860 -25760
- Misses 4123 29887 +25764
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #26937 +/- ##
==========================================
- Coverage 91.87% 91.87% -0.01%
==========================================
Files 180 180
Lines 50746 50752 +6
==========================================
+ Hits 46623 46626 +3
- Misses 4123 4126 +3
Continue to review full report at Codecov.
|
ghost
changed the title
Interval.overlaps handling of empty intervalsoverlaps handling of empty intervals in INterval and IntervalArray
ghost
changed the title
overlaps handling of empty intervals in INterval and IntervalArrayoverlaps handling of empty intervals in Interval and IntervalArray
|
I would have liked to have an |
|
ready for review. CC @TomAugspurger , @jschendel . |
pandas/_libs/interval.pyx
Outdated
| @@ -423,7 +423,15 @@ cdef class Interval(IntervalMixin): | |||
| msg = '`other` must be an Interval, got {other}' | |||
| raise TypeError(msg.format(other=type(other).__name__)) | |||
|
|
|||
| # TODO:: use Interval.is_empty() if/when implemented | |||
| def is_empty(intv): | |||
There was a problem hiding this comment.
needs a doc-string / typing. I think we should just make this a proper method, with proper tests in this PR.
pandas/core/arrays/interval.py
Outdated
| @@ -1074,14 +1074,26 @@ def overlaps(self, other): | |||
| msg = '`other` must be Interval-like, got {other}' | |||
| raise TypeError(msg.format(other=type(other).__name__)) | |||
|
|
|||
| # TODO:: use Interval.is_empty() if/when implemented | |||
| ((0, 1, "both"), (0.4, 0.6, "neither"), True), | ||
| ((0, 1, "neither"), (0.4, 0.6, "neither"), True), | ||
| ]) | ||
| def overlaps_cases(request): |
| import pandas.util.testing as tm | ||
|
|
||
| # silence "imported but unused" warning by flake8 | ||
| overlaps_cases | ||
|
|
There was a problem hiding this comment.
you are missing the point, you need to move the conftest to a higher level to avoid this import smell
There was a problem hiding this comment.
Shit. Here we go again.
@jreback , this is the second time you've told me "I'm missing the point", assuming I'm a moron, and been monumentally dead wrong in both times and on both counts. It's also the second PR I'm going to abandon because of your patronizing and offensive manner.
I've asked for you to stay away from my PRs and leave review to saner team members. But here we are again, with you talking shit, and me not willing to take it.
There is no sane place to put the conftest because of the way the test hierarchy is built.
To share a conftest between pandas/tests/arrays/interval and pandas/tests/scalar/interval
It would have to be placed in pandas/tests and then be loaded by dozens of unrelated modules. That's far worse than these silly two lines. But for you its obvious that other developers couldn't possibly apply any judgment, and you have have to micromanage them to the point of disgust.
The hell with this.
@pandas-dev Your project has a problem. jreback should not be allowed near contributors.
ghost
closed this
|
@pilkibun your attitude is about the worst I have ever seen in open source. Just because you don't like my comments does not reward behavior like this. good luck. |
ghost
deleted the
This supercedes #26894, which made
Interval()raise an exception on attempts to create of the form[x,x)or(x,x].git diff upstream/master -u -- "*.py" | flake8 --diffDiscussion in #26893 suggests we may want to treat empty intervals as Singletons in the future. But for now, just fix overlaps.
update:
IntervalArrayhas its own vectorized implementation ofoverlaps, fixed and now shares the new test cases I added forInterval.overlaps.