Skip to content

Removed dead intervaltree code #30459

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

Merged
merged 1 commit into from
Dec 26, 2019

Conversation

WillAyd
Copy link
Member

@WillAyd WillAyd commented Dec 24, 2019

While looking to reduce build warnings I came across these methods and couldn't find any public use to them. The only one that seems to get hit is get_loc and that is in tests only so I think dead code

@jbrockmendel
Copy link
Member

It looks like IntervalIndex.get_loc doesn't use self._engine.get_loc, which seems kind of weird. Also looks like there's a good bit of either untested or dead code in indexes.interval: https://codecov.io/gh/pandas-dev/pandas/src/master/pandas/core/indexes/interval.py

@WillAyd
Copy link
Member Author

WillAyd commented Dec 24, 2019

@jschendel

@jreback jreback added this to the 1.0 milestone Dec 25, 2019
@jreback jreback added the Interval Interval data type label Dec 25, 2019
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

lgtm. @jschendel if you'd have a look and merge if ok.

@jschendel
Copy link
Member

It looks like IntervalIndex.get_loc doesn't use self._engine.get_loc, which seems kind of weird.

This is a result of the interval indexing behavior changes where we no longer allow partial interval overlaps, which was the main purpose of IntervalTree.get_loc. The logic is now a bit simpler and much more straightforward to vectorize in numpy.

Also looks like there's a good bit of either untested or dead code in indexes.interval

Yes, some of these can be removed for the same reason as above. However, others are simply helper functions that are poorly tested. After a quick glance:

  • _get_next_label is used but poorly tested
  • _get_interval_closed_bounds can be removed
  • _maybe_cast_indexed can be removed
  • _can_reindex is used and overrides a parent class definition that's not valid for IntervalIndex
  • _find_non_overlapping_monotonic_bounds can be removed

Can look into the above more next week.

@jschendel jschendel merged commit cde73af into pandas-dev:master Dec 26, 2019
@jschendel
Copy link
Member

Thanks @WillAyd!

AlexKirko pushed a commit to AlexKirko/pandas that referenced this pull request Dec 29, 2019
@WillAyd WillAyd deleted the intervaltree-remove-methods branch January 16, 2020 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Interval Interval data type
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants