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

ENH: Support For Interval __contains__ Other Interval (#46613) #47927

Merged
merged 12 commits into from Aug 15, 2022

Conversation

kapiliyer
Copy link
Contributor

@@ -36,10 +36,6 @@ def test_contains(self, interval):
assert 1 in interval
assert 0 not in interval

msg = "__contains__ not defined for two intervals"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed by this enhancement.

doc/source/whatsnew/v1.5.0.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@tgy tgy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Need to fix the tests now

@hdrodz
Copy link

hdrodz commented Aug 4, 2022

Apologies for the mess in the history! I suggested @kapiliyer rebase rather than merge and I'm seeing now that the history's in a weird state. Later today I'll work with him to get his branch into a cleaner state.

@hdrodz
Copy link

hdrodz commented Aug 5, 2022

History should be in a saner state now. Apologies again for the trouble!

Copy link
Contributor

@tgy tgy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@@ -410,7 +412,17 @@ cdef class Interval(IntervalMixin):

def __contains__(self, key) -> bool:
if _interval_like(key):
raise TypeError("__contains__ not defined for two intervals")
key_closed_left = key.inclusive in ('left', 'both')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @venaturum if you'd like to review if this aligns with piso

@mroeschke mroeschke added the Interval Interval data type label Aug 8, 2022
@kapiliyer kapiliyer closed this Aug 15, 2022
@kapiliyer kapiliyer reopened this Aug 15, 2022
@mroeschke mroeschke added this to the 1.5 milestone Aug 15, 2022
@mroeschke mroeschke merged commit d0bd469 into pandas-dev:main Aug 15, 2022
@mroeschke
Copy link
Member

Thanks @kapiliyer!

YYYasin19 pushed a commit to YYYasin19/pandas that referenced this pull request Aug 23, 2022
…) (pandas-dev#47927)

* ENH: Support For Interval __contains__ Other Interval (pandas-dev#46613)

* ENH: Support For Interval __contains__ Other Interval (pandas-dev#46613)

* Update doc/source/whatsnew/v1.5.0.rst

Co-authored-by: Valentin Iovene <valentin@too.gy>

* ENH: Support For Interval __contains__ Other Interval (pandas-dev#46613)

* ENH: Support For Interval __contains__ Other Interval (pandas-dev#46613)

* Fix: Unintentionally Modified Range

* ENH: Support For Interval __contains__ Other Interval (pandas-dev#46613)

* Fix: Unintentionally Modified Range

* ENH: Support For Interval __contains__ Other Interval (pandas-dev#46613)

Co-authored-by: Valentin Iovene <valentin@too.gy>
noatamir pushed a commit to noatamir/pandas that referenced this pull request Nov 9, 2022
…) (pandas-dev#47927)

* ENH: Support For Interval __contains__ Other Interval (pandas-dev#46613)

* ENH: Support For Interval __contains__ Other Interval (pandas-dev#46613)

* Update doc/source/whatsnew/v1.5.0.rst

Co-authored-by: Valentin Iovene <valentin@too.gy>

* ENH: Support For Interval __contains__ Other Interval (pandas-dev#46613)

* ENH: Support For Interval __contains__ Other Interval (pandas-dev#46613)

* Fix: Unintentionally Modified Range

* ENH: Support For Interval __contains__ Other Interval (pandas-dev#46613)

* Fix: Unintentionally Modified Range

* ENH: Support For Interval __contains__ Other Interval (pandas-dev#46613)

Co-authored-by: Valentin Iovene <valentin@too.gy>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Interval Interval data type
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ENH: Interval.__contains__(self, other: Interval)
4 participants