ENH: Implement overlaps method for Interval-like #22939

jschendel · 2018-10-02T03:10:44Z

tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry
xref ENH: Interval type should support intersection, union & overlaps & difference #21998, Add spec for new Interval / IntervalIndex methods: .overlaps(), .covers() #18975

Implements Interval.overlaps, IntervalArray.overlaps, and IntervalIndex.overlaps.

Implementing this so there's an interval method that can be tested in the interval accessor PR (#19502), as there are currently only attributes that can be used by the accessor. Planning to reboot that PR soon after this is merged.

I didn't implement IntervalArray.overlaps(IntervalArray); the PR providing the specs for overlaps was recently closed due to being stale, and I don't recall there being a resolution on this behavior.

pep8speaks · 2018-10-02T03:10:50Z

Hello @jschendel! Thanks for updating the PR.

There are no PEP8 issues in the file pandas/conftest.py !
There are no PEP8 issues in the file pandas/core/arrays/interval.py !
There are no PEP8 issues in the file pandas/core/indexes/interval.py !
There are no PEP8 issues in the file pandas/tests/arrays/interval/test_interval_ops.py !
There are no PEP8 issues in the file pandas/tests/scalar/interval/test_interval_ops.py !

Comment last updated on October 02, 2018 at 03:16 Hours UTC

jschendel · 2018-10-02T03:22:54Z

pandas/tests/arrays/interval/test_interval_ops.py

+    """
+    Fixture for IntervalArray and IntervalIndex class constructors
+    """
+    return request.param


Combined the IntervalArray and IntervalIndex tests here, as the behavior for both should be identical. Not super happy about having the IntervalIndex tests here though, since this is in the arrays directory, but seemed better than creating dupe tests for IntervalIndex. Could maybe move these tests into a common location and combine with the Interval tests, but not exactly sure what would be best.

u can make a common mixin class where the tests live that doesn’t start with test_ then use it in both places. we do this with the parser tests for example.

this pattern is going to become more common as we use more EA for Indexes

so happy to have this change

done (see my latest comment for additional details)

codecov · 2018-10-02T09:30:27Z

Codecov Report

Merging #22939 into master will increase coverage by <.01%.
The diff coverage is 93.33%.

@@            Coverage Diff             @@
##           master   #22939      +/-   ##
==========================================
+ Coverage   92.19%   92.19%   +<.01%     
==========================================
  Files         169      169              
  Lines       50833    50966     +133     
==========================================
+ Hits        46863    46987     +124     
- Misses       3970     3979       +9

Flag	Coverage Δ
#multiple	`90.61% <93.33%> (ø)`	⬆️
#single	`42.27% <40%> (-0.1%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/arrays/interval.py	`93.02% <100%> (+0.46%)`	⬆️
pandas/core/indexes/interval.py	`94.65% <80%> (+0.49%)`	⬆️
pandas/plotting/_compat.py	`87.5% <0%> (-3.81%)`	⬇️
pandas/core/ops.py	`94.63% <0%> (-2.74%)`	⬇️
pandas/compat/numpy/function.py	`86.66% <0%> (-1.82%)`	⬇️
pandas/core/internals/concat.py	`96.83% <0%> (-1.54%)`	⬇️
pandas/core/dtypes/concat.py	`97.69% <0%> (-1.49%)`	⬇️
pandas/compat/numpy/__init__.py	`92.85% <0%> (-1.09%)`	⬇️
pandas/core/internals/blocks.py	`92.91% <0%> (-0.95%)`	⬇️
pandas/core/internals/managers.py	`95.77% <0%> (-0.9%)`	⬇️
... and 60 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5ce06b5...a765e93. Read the comment docs.

jorisvandenbossche

Didn't look in detail yet, some quick comments

pandas/_libs/interval.pyx

jorisvandenbossche · 2018-10-04T06:55:02Z

pandas/core/arrays/interval.py

+
+        Parameters
+        ----------
+        other : Interval-like


Why Interval-like and not Interval ?

Changed. Originally wrote Interval-like to mean Interval/IntervalIndex/IntervalArray, as there were originally plans to support all three. Probably best to just say Interval for now since that's the only thing supported, and leave such a modification for when the other two are implemented.

jorisvandenbossche · 2018-10-04T06:55:33Z

pandas/core/arrays/interval.py

@@ -1015,6 +1017,40 @@ def repeat(self, repeats, **kwargs):
        right_repeat = self.right.repeat(repeats, **kwargs)
        return self._shallow_copy(left=left_repeat, right=right_repeat)

+    _interval_shared_docs['overlaps'] = """\
+        Check elementwise whether the given input overlaps the intervals in


First sentence should be a one-liner

pandas/core/arrays/interval.py

jschendel · 2018-10-14T21:10:28Z

Created a common mixin class for the tests:

Put the mixin class in tests/arrays/interval/interval_ops.py
- Not sure that this is the best location; any better alternatives?
- Kind of gross import in the indexes tests: from ...arrays.interval.interval_ops
Created test_interval_ops.py files in both the indexes and arrays subfolders that use the mixin.

jreback · 2018-10-15T12:41:13Z

pandas/tests/arrays/interval/test_interval_ops.py

@@ -0,0 +1,15 @@
+"""Tests for Interval-Interval operations, such as overlaps, contains, etc."""


test_ops is ok

jreback · 2018-10-15T12:41:26Z

pandas/tests/indexes/interval/test_interval_ops.py

@@ -0,0 +1,15 @@
+"""Tests for Interval-Interval operations, such as overlaps, contains, etc."""
+import pytest


jreback · 2018-10-15T12:41:43Z

pandas/tests/indexes/interval/test_interval_ops.py

+import pytest
+
+from pandas import IntervalIndex
+from ...arrays.interval.interval_ops import BaseOverlaps


use absolute imports. do we really need a mixin? (as opposed to fixtures)?

Reverted back to using a fixture; the downside of using a fixture to test both is that you end up with IntervalIndex tests in the array subfolder which is unintuitive. The mixin admittedly adds some code bloat, so not really an ideal solution either.

jreback · 2018-10-15T12:41:50Z

pandas/tests/scalar/interval/test_interval_ops.py

@@ -0,0 +1,61 @@
+"""Tests for Interval-Interval operations, such as overlaps, contains, etc."""
+import pytest
+


jreback · 2018-10-15T12:42:09Z

pandas/tests/scalar/interval/test_interval_ops.py

+    """
+    return request.param
+
+


i don't think this needs a class

My plan is for this for all interval-interval specific methods to be tested here (e.g. covers, adjacent, etc.). Agreed that as-is this shouldn't need a class, but once additional methods are added it seems cleanest to organize each one in a separate class. Can get rid of the class if you'd still prefer that approach though.

jreback · 2018-10-18T15:57:27Z

lgtm. if you have any other comments @jorisvandenbossche or @TomAugspurger otherwise merge away.

jschendel · 2018-10-24T02:05:20Z

ping @jorisvandenbossche @TomAugspurger : I'm planning to merge this within the next couple days so I can have a method to test out in the interval accessor PR. Just wanted to give both of you a quick heads up in case either of you wanted to review prior to this getting merged. The interval accessor PR has some merge conflicts that need to be resolved, so no immediate rush, but I'm planning to address the conflicts soon.

TomAugspurger · 2018-10-24T11:14:24Z

doc/source/whatsnew/v0.24.0.txt

@@ -194,6 +194,7 @@ Other Enhancements
 - :meth:`Index.to_frame` now supports overriding column name(s) (:issue:`22580`).
 - New attribute :attr:`__git_version__` will return git commit sha of current build (:issue:`21295`).
 - Compatibility with Matplotlib 3.0 (:issue:`22790`).
+- Added :meth:`Interval.overlaps`, :meth:`IntervalArray.overlaps`, and :meth:`IntervalIndex.overlaps` for determining overlaps between interval-like objects (:issue:`21998`)


I don't think IntervalArray is in our API docs, so that link will cause a warning.

It is not in the API docs at this time, but I imagine we'd add it if we decide to make IntervalArray (and other EA's) public? I have a few doc related TODOs surrounding whether or not IntervalArray gets made public, so I'll make an issue for these TODOs and include the above in the next day or so.

TomAugspurger · 2018-10-24T11:21:26Z

pandas/core/arrays/interval.py

+    @Appender(_interval_shared_docs['overlaps'] % _shared_docs_kwargs)
+    def overlaps(self, other):
+        if isinstance(other, (IntervalArray, ABCIntervalIndex)):
+            raise NotImplementedError


Do we intend to eventually support IntervalArray.overlaps(IntervalArray), where the arrays are the same shape?

There was discussion about this in #18975 but no consensus on the behavior. I'd be open to implementing it if there is demand. I could see adding a how parameter that accepts {'any', 'all', 'pairwise'} to determine the behavior (e.g. for 'any'/'all' the boolean indicates if the given interval overlaps any/all of the intervals in the supplied array, and 'pairwise' requiring the lengths to be the same and indicating if the nth element overlaps the nth element of the supplied array.

I don't have a use case for IntervalArray.overlaps(IntervalArray) though, so not entirely sure what portions of the above are practically useful, if any, or if some other behavior would be more commonly used. The 'pairwise' option would be straightforward to implement, with the other two maybe requiring some care to implement in a performant manner.

jreback · 2018-10-24T12:19:07Z

thanks @jschendel pls follow up as needed.

ENH: Implement overlaps method for Interval-like

e5be997

jschendel added Enhancement Interval Interval data type labels Oct 2, 2018

jschendel added this to the 0.24.0 milestone Oct 2, 2018

pep8

2d240f2

jschendel commented Oct 2, 2018

View reviewed changes

jorisvandenbossche reviewed Oct 4, 2018

View reviewed changes

jschendel added 2 commits October 6, 2018 12:00

expand documentation

21e0f49

mixin class for tests

69024a2

jreback requested changes Oct 15, 2018

View reviewed changes

jschendel added 4 commits October 16, 2018 00:20

clean docstrings

acfb2c1

rename test_interval_ops -> test_ops

98b847e

revert to fixture instead of mixin for tests

96497ec

lint

a765e93

jreback approved these changes Oct 18, 2018

View reviewed changes

TomAugspurger reviewed Oct 24, 2018

View reviewed changes

TomAugspurger approved these changes Oct 24, 2018

View reviewed changes

jreback merged commit 95aab41 into pandas-dev:master Oct 24, 2018

jschendel deleted the enh-interval-overlaps branch October 25, 2018 02:07

tm9k1 pushed a commit to tm9k1/pandas that referenced this pull request Nov 19, 2018

ENH: Implement overlaps method for Interval-like (pandas-dev#22939)

2ca401e

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

ENH: Implement overlaps method for Interval-like (pandas-dev#22939)

d575b6d

sterlinm mentioned this pull request Aug 14, 2021

ENH: IntervalArray/IntervalIndex Range Joins #43031

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: Implement overlaps method for Interval-like #22939

ENH: Implement overlaps method for Interval-like #22939

jschendel commented Oct 2, 2018 •

edited

Loading

pep8speaks commented Oct 2, 2018 •

edited

Loading

jschendel Oct 2, 2018

jreback Oct 4, 2018 •

edited

Loading

jschendel Oct 14, 2018

codecov bot commented Oct 2, 2018 •

edited

Loading

jorisvandenbossche left a comment

jorisvandenbossche Oct 4, 2018

jschendel Oct 6, 2018

jorisvandenbossche Oct 4, 2018

jschendel Oct 6, 2018

jschendel commented Oct 14, 2018

jreback Oct 15, 2018

jschendel Oct 18, 2018

jreback Oct 15, 2018

jschendel Oct 18, 2018

jreback Oct 15, 2018

jschendel Oct 18, 2018 •

edited

Loading

jreback Oct 15, 2018

jschendel Oct 18, 2018

jreback Oct 15, 2018

jschendel Oct 18, 2018

jreback commented Oct 18, 2018

jschendel commented Oct 24, 2018

TomAugspurger Oct 24, 2018

jschendel Oct 25, 2018

TomAugspurger Oct 24, 2018

jschendel Oct 25, 2018

jreback commented Oct 24, 2018

		@@ -0,0 +1,15 @@
		"""Tests for Interval-Interval operations, such as overlaps, contains, etc."""

		@@ -0,0 +1,15 @@
		"""Tests for Interval-Interval operations, such as overlaps, contains, etc."""
		import pytest

		@@ -0,0 +1,61 @@
		"""Tests for Interval-Interval operations, such as overlaps, contains, etc."""
		import pytest

ENH: Implement overlaps method for Interval-like #22939

ENH: Implement overlaps method for Interval-like #22939

Conversation

jschendel commented Oct 2, 2018 • edited Loading

pep8speaks commented Oct 2, 2018 • edited Loading

Comment last updated on October 02, 2018 at 03:16 Hours UTC

Choose a reason for hiding this comment

jreback Oct 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Oct 2, 2018 • edited Loading

Codecov Report

jorisvandenbossche left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jschendel commented Oct 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jschendel Oct 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Oct 18, 2018

jschendel commented Oct 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Oct 24, 2018

jschendel commented Oct 2, 2018 •

edited

Loading

pep8speaks commented Oct 2, 2018 •

edited

Loading

jreback Oct 4, 2018 •

edited

Loading

codecov bot commented Oct 2, 2018 •

edited

Loading

jschendel Oct 18, 2018 •

edited

Loading