Skip to content

New Interval / IntervalIndex behavior spec #16386

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.

Sign up for GitHub

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 63 commits into from
Dec 28, 2017
+588 −0
Merged

New Interval / IntervalIndex behavior spec #16386

Changes from 1 commit
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
18c76f4
new interval and intervalIndex behavior spec
May 18, 2017
ddc508c
moved tests from indexing to indexes, and added new tests
May 25, 2017
a1c3e7a
first pass at @shoyer's review
May 26, 2017
557d701
quick update comments
May 26, 2017
7d28038
another quick pass
Jun 6, 2017
ff1fbf2
sanity check
Jun 6, 2017
aadfdcd
fixed one little issue.
Jun 7, 2017
c7f6fb8
fixed most of @jorisvandenbossche's review
Jun 19, 2017
1379b08
worked on 'contains'
Jun 19, 2017
80ebeb3
remove covers and overlaps -- to be added to a separate PR
Jun 19, 2017
fef3187
take back changes involving .contains() and __contains__
Jun 20, 2017
e3a12fa
some updates discussed
Jun 22, 2017
06a2835
quick thought...
Jun 22, 2017
08bd9e4
tiny update
Jun 29, 2017
6381744
An update to get_indexer behavior
Jul 6, 2017
b346af7
update slice locs behavior
Jul 6, 2017
ccd23aa
remove tests flagged for removal.
Jul 14, 2017
ac818f9
left side of interval must be <= right side
Jul 17, 2017
4a10007
move to throwing errors for slice locs with overlapping stuff
Jul 17, 2017
e549c3d
compress test cases
Jul 17, 2017
d5a8287
Change to InvalidIndexError
Jul 17, 2017
0e50729
compressing tests even more
Jul 17, 2017
2c953b6
some lint issues
Jul 17, 2017
08d315c
I believe all the test cases are complete.
Jul 18, 2017
9f905a8
autopep
Jul 18, 2017
e1eeb59
more pep8
Jul 18, 2017
4c54f33
remove some stuff
Jul 18, 2017
ca04cb2
tiny update
Jul 18, 2017
bfaefef
update get_indexer_non_unique tests
Jul 26, 2017
ce5074a
autopep
Jul 26, 2017
4c5496e
hopefully this is valid python
Aug 3, 2017
33938d6
Merge remote-tracking branch 'pandas-dev/master'
Sep 15, 2017
7f4c5e5
a couple updates, and a bug fix
Sep 15, 2017
0aaaddf
oops
Sep 15, 2017
a44c926
fix some lint issues
Sep 15, 2017
4cef040
start working on the review
Sep 15, 2017
66486d0
fix some slice-locs stuff
Sep 16, 2017
c0dfef8
lint issues mostly
Sep 16, 2017
5301dd5
try to satisfy pep
Sep 16, 2017
57c9ba7
for some reason that didn't make PEP happy
Sep 16, 2017
4cbbf2d
pep
Sep 16, 2017
6126662
parameterize and add a couple comments.
Sep 25, 2017
dc00af6
hopefully this works.
Sep 26, 2017
604d48d
Merge remote-tracking branch 'pandas-dev/master'
Sep 26, 2017
10c9177
fix merge conflicts
Sep 26, 2017
c8be3e6
parametrize a little bit more
jorisvandenbossche Sep 29, 2017
93f1742
remove _updated_behaviour from test name + PEP8 (unused imports)
jorisvandenbossche Sep 29, 2017
3464883
update loc tests
jorisvandenbossche Sep 30, 2017
06484f5
Merge branch 'master' into master
alexlenail Dec 19, 2017
e82ae33
update comments
Dec 20, 2017
6834c9a
Merge remote-tracking branch 'origin/master'
Dec 20, 2017
33ebe1b
worked on parameterization
Dec 20, 2017
be050d6
move xfails to top of module
Dec 21, 2017
1cc8004
minor update for pep
Dec 21, 2017
930da2b
more for pep
Dec 21, 2017
2e01d28
fix mistake
Dec 21, 2017
5c000a0
might have messed up a function signature during a previous merge. Se…
Dec 22, 2017
96c978a
formatting, potentially last bug
Dec 22, 2017
ae03d01
Merge branch 'master' into PR_TOOL_MERGE_PR_16386
jreback Dec 23, 2017
518d16e
skip interval_new tests
jreback Dec 23, 2017
f530637
lint
jreback Dec 23, 2017
d90d310
Merge branch 'master' into PR_TOOL_MERGE_PR_16386
jreback Dec 23, 2017
acefcb0
Merge branch 'master' into PR_TOOL_MERGE_PR_16386
jreback Dec 28, 2017
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
compress test cases
  • Loading branch information
Alexander Lenail committed Jul 17, 2017
commit e549c3dff59409d8395bae22a08fbada1011f6b3
203 changes: 47 additions & 156 deletions pandas/tests/indexes/test_interval.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,6 @@ def slice_locs_with_interval_updated_behavior(self):
assert index.slice_locs(end=Interval(0, 2)) == ()
assert index.slice_locs(start=Interval(2, 4), end=Interval(0, 2)) == ()


@pytest.mark.xfail(reason="new indexing tests for issue 16316")
def slice_locs_with_ints_and_floats_updated_behavior(self):

Expand Down Expand Up @@ -654,187 +653,79 @@ def get_indexer_for_interval_updated_behavior(self):
index = IntervalIndex.from_tuples([(0, 2.5), (1, 3), (2, 4)], closed='right')

# single queries
Copy link
Contributor

@jreback jreback Sep 23, 2017

Choose a reason for hiding this comment

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

same
result = index.get_indexer([Interval(1, 3, closed='right')])
expected = np.array([1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer([Interval(1, 3, closed='left')])
expected = np.array([-1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer([Interval(1, 3, closed='both')])
expected = np.array([-1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer([Interval(1, 3, closed='neither')])
expected = np.array([-1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer([Interval(1, 4, closed='right')])
expected = np.array([-1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer([Interval(0, 4, closed='right')])
expected = np.array([-1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer([Interval(1, 2, closed='right')])
expected = np.array([-1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)
queries = [ Interval(1, 3, closed='right'),
Interval(1, 3, closed='left'),
Interval(1, 3, closed='both'),
Interval(1, 3, closed='neither'),
Interval(1, 4, closed='right'),
Interval(0, 4, closed='right'),
Interval(1, 2, closed='right') ]
expected = [1, -1, -1, -1, -1, -1, -1]

for query, expected_result in zip(queries, expected):
result = index.get_indexer([query])
expect = np.array([expected_result], dtype='intp')
tm.assert_numpy_array_equal(result, expect)

# multiple queries
result = index.get_indexer([Interval(2, 4, closed='right'),
Interval(1, 3, closed='right') ])
expected = np.array([2, 1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer([Interval(1, 3, closed='right'),
Interval(0, 2, closed='right') ])
expected = np.array([1, -1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)
queries = [
[Interval(2, 4, closed='right'), Interval(1, 3, closed='right')],
[Interval(1, 3, closed='right'), Interval(0, 2, closed='right')],
[Interval(1, 3, closed='right'), Interval(1, 3, closed='left')],
index ]
expected = [[2, 1], [1, -1], [1, -1], [0, 1, 2]]

result = index.get_indexer([Interval(1, 3, closed='right'),
Interval(1, 3, closed='left') ])
expected = np.array([1, -1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)
for query, expected_result in zip(queries, expected):
result = index.get_indexer(query)
expect = np.array(expected_result, dtype='intp')
tm.assert_numpy_array_equal(result, expect)

result = index.get_indexer(index)
expected = np.array([0, 1, 2], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

@pytest.mark.xfail(reason="new indexing tests for issue 16316")
def get_indexer_for_ints_and_floats_updated_behavior(self):

Copy link
Contributor

@jreback jreback Sep 23, 2017

Choose a reason for hiding this comment

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

same; anytime you are doing a loop you almost always want to parametrize
Copy link
Contributor

@jreback jreback Sep 23, 2017

Choose a reason for hiding this comment

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

just a little tidbit on why.

by putting everything in a loop, you MUST debug in order and you have no idea what currently works and what does not. parametrize basically makes little tests out of everything, so it is very easy to see where things work and where they don't. furthermore you can then debug with a very very narrow test and get to exactly what you need w/o going thru lots of steps.
index = IntervalIndex.from_tuples([(0, 1), (1, 2), (3, 4)], closed='right')

# single queries
result = index.get_indexer([-0.5])
expected = np.array([-1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer([0])
expected = np.array([-1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer([0.5])
expected = np.array([0], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer([1])
expected = np.array([0], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer([1.5])
expected = np.array([1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer([2])
expected = np.array([1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer([2.5])
expected = np.array([-1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer([3])
expected = np.array([-1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)
queries = [-0.5, 0, 0.5, 1, 1.5, 2, 2.5, 3, 3.5, 4, 4.5]
expected = [-1, -1, 0, 0, 1, 1, -1, -1, 2, 2, -1]

result = index.get_indexer([3.5])
expected = np.array([2], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer([4])
expected = np.array([2], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer([4.5])
expected = np.array([-1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)
for query, expected_result in zip(queries, expected):
result = index.get_indexer([query])
expect = np.array([expected_result], dtype='intp')
tm.assert_numpy_array_equal(result, expect)

# multiple queries
result = index.get_indexer([1, 2])
expected = np.array([0, 1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer([1, 2, 3])
expected = np.array([0, 1, -1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer([1, 2, 3, 4])
expected = np.array([0, 1, -1, 2], dtype='intp')
tm.assert_numpy_array_equal(result, expected)
queries = [[1, 2],[1, 2, 3],[1, 2, 3, 4],[1, 2, 3, 4, 2]]
expected = [[0, 1], [0, 1, -1], [0, 1, -1, 2], [0, 1, -1, 2, 1]]

result = index.get_indexer([1, 2, 3, 4, 2])
expected = np.array([0, 1, -1, 2, 1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)
for query, expected_result in zip(queries, expected):
result = index.get_indexer(query)
expect = np.array(expected_result, dtype='intp')
tm.assert_numpy_array_equal(result, expect)

Copy link
Member

@jorisvandenbossche jorisvandenbossche Sep 15, 2017

Choose a reason for hiding this comment

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

Should there be a test for overlapping IntervalIndex as well? (I think we said it should raise in that case, but check the discussion)
Copy link
Contributor Author

@alexlenail alexlenail Sep 16, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@shoyer believed this case should raise. The current behavior in pandas 20.2 is essentially what you would expect from get_indexer_non_unique:

>>> index = pd.IntervalIndex.from_tuples([(0, 2), (1, 3), (2, 4)]) >>> index.get_indexer([1]) array([0]) >>> index.get_indexer([2]) array([0, 1])

What do you think I should write in here?
Copy link
Member

@jorisvandenbossche jorisvandenbossche Sep 16, 2017

Choose a reason for hiding this comment

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

I agree that it should raise, so I would just add a test for it to make sure we don't lose that discussion.
@pytest.mark.xfail(reason="new indexing tests for issue 16316")
def get_indexer_nonunique_for_ints_and_floats_updated_behavior(self):

index = IntervalIndex.from_tuples([(0, 2.5), (1, 3), (2, 4)], closed='left')

# single queries
result = index.get_indexer_nonunique([-0.5])
expected = np.array([-1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer_nonunique([0])
expected = np.array([0], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer_nonunique([0.5])
expected = np.array([0], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer_nonunique([1])
expected = np.array([0, 1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer_nonunique([1.5])
expected = np.array([0, 1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)
queries = [-0.5, 0, 0.5, 1, 1.5, 2, 2.5, 3, 3.5, 4, 4.5]
expected = [[-1], [0], [0], [0, 1], [0, 1], [0, 1, 2], [1, 2], [2], [2], [-1], [-1]]

result = index.get_indexer_nonunique([2])
expected = np.array([0, 1, 2], dtype='intp')
tm.assert_numpy_array_equal(result, expected)
for query, expected_result in zip(queries, expected):
result = index.get_indexer_nonunique([query])
Copy link
Member

@shoyer shoyer Jul 25, 2017

Choose a reason for hiding this comment

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

It's actually get_indexer_non_unique().

Also, it returns a tuple of indexer and indices for missing elements, e.g., try

idx = pd.Index([0, 0, 1, 2, 2]) idx.get_indexer_non_unique([0, 2, 3])
Copy link
Contributor Author

@alexlenail alexlenail Jul 25, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

The result of that call is

(Int64Index([0, 1, 3, 4, -1], dtype='int64'), array([2]))

i.e.

([0, 1, 3, 4, -1], [2])

But that's fairly confusing to me. Wouldn't you instead want

([0, 1], [3, 4], [-1])

That would make more sense to me. What was the motivation here?
Copy link
Member

@shoyer shoyer Jul 25, 2017

Choose a reason for hiding this comment

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

The return value looks strange to me, too, but fixing it is out of scope here :)
Copy link
Contributor Author

@alexlenail alexlenail Jul 25, 2017

Choose a reason for hiding this comment

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

Mind if I open another issue and then also a PR to fix that? (after this one is done?)
Copy link
Member

@shoyer shoyer Jul 25, 2017

Choose a reason for hiding this comment

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

Sure, please do open another issue to discuss. To be clear: I don't understand the exact motivation here, either, so we'll want to hear from Jeff on this one. One thing to consider is that unstructured lists of lists like [[0, 1], [3, 4], [-1]] can't be made used very efficiently, because they can't be represented by numpy arrays.
Copy link
Contributor

@jreback jreback Jul 25, 2017

Choose a reason for hiding this comment

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

this is an indexer to the original array

you can .take(..) to get the result (excluding -1 which are missing values ; there position marks where things are missing)
expect = np.array(expected_result, dtype='intp')
tm.assert_numpy_array_equal(result, expect)

result = index.get_indexer_nonunique([2.5])
expected = np.array([1, 2], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer_nonunique([3])
expected = np.array([2], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer_nonunique([3.5])
expected = np.array([2], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer_nonunique([4])
expected = np.array([-1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer_nonunique([4.5])
expected = np.array([-1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)
queries = [[1, 2], [1, 2, 3], [1, 2, 3, 4], [1, 2, 3, 4, 2]]
expected = [[0, 1, 0, 1, 2], [0, 1, 0, 1, 2, 2], [0, 1, 0, 1, 2, 2, -1], [0, 1, 0, 1, 2, 2, -1, 0, 1, 2]] # should we use tuples here?

# multiple queries
result = index.get_indexer_nonunique([1, 2])
expected = np.array([0, 1, 0, 1, 2], dtype='intp') # maybe we could put these in tuples? [(0, 1), (0, 1, 2)]
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer_nonunique([1, 2, 3])
expected = np.array([0, 1, 0, 1, 2, 2], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer_nonunique([1, 2, 3, 4])
expected = np.array([0, 1, 0, 1, 2, 2, -1], dtype='intp')
tm.assert_numpy_array_equal(result, expected)

result = index.get_indexer_nonunique([1, 2, 3, 4, 2])
expected = np.array([0, 1, 0, 1, 2, 2, -1, 0, 1, 2], dtype='intp')
tm.assert_numpy_array_equal(result, expected)
for query, expected_result in zip(queries, expected):
result = index.get_indexer_nonunique(query)
expect = np.array(expected_result, dtype='intp')
tm.assert_numpy_array_equal(result, expect)

# we may also want to test get_indexer for the case when
# the intervals are duplicated, decreasing, non-monotonic, etc..
Expand Down