BUG: MultiIndex intersection with sort=False does not preserve order

[jeffzi]

• closes BUG: MultiIndex intersection with sort=False does not preserve order #31325
• tests added / passed
• passes black pandas
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

The intersection of 2 MultiIndex with sort=False does not preserve the order, whereas Index. intersection() does. This behavior does not seem to be intentional since the tests for difference and symmetric_difference are testing for order preservation.

For example:

import pandas as pd

arrays = [['bar', 'bar', 'baz', 'baz', 'foo', 'foo', 'qux', 'qux'],
          ['one', 'two', 'one', 'two', 'one', 'two', 'one', 'two']]
tuples = list(zip(*arrays))
idx = pd.MultiIndex.from_tuples(tuples, names=['first', 'second'])

left = idx[2::-1]
print(left)
#> MultiIndex([('baz', 'one'),
#>             ('bar', 'two'),
#>             ('bar', 'one')],
#>            names=['first', 'second'])
right = idx[:5]
print(right)
#> MultiIndex([('bar', 'one'),
#>             ('bar', 'two'),
#>             ('baz', 'one'),
#>             ('baz', 'two'),
#>             ('foo', 'one')],
#>            names=['first', 'second'])

# expected same order as left
intersect = left.intersection(right, sort=False)
print(intersect)
#> MultiIndex([('bar', 'two'),
#>             ('baz', 'one'),
#>             ('bar', 'one')],
#>            names=['first', 'second'])

^{Created on 2020-01-25 by the reprexpy package}

I verified that my PR does not decrease performances.

I also modified the test for union. The implementation was preserving the order with sort=False but the tests were not verifying it.

[charlesdong1991]

Very nice! @jeffzi

could you pls also create an issue and reference this PR to the issue?

[jeffzi]

I benchmarked the intersection of 2 arrays of tuples: master Vs this PR Vs suggestion by @jreback to take inspiration of difference.

I also looked at the implementation of Index.intersection and found that performances can be increased in the case of 2 monotonic MultiIndex.

@jreback If that's ok with you, I can commit the "mixed" version below.

iimport numpy as np
import pandas as pd
from pandas.testing import assert_index_equal

def master(self, other):
    return set(self._ndarray_values) & set(other._ndarray_values)

def pr(self, other):
    lvals = self._ndarray_values
    rvals = other._ndarray_values
    
    other_uniq = set(rvals)
    seen = set()
    return [x for x in lvals if x in other_uniq and not (x in seen or seen.add(x))]

def indexer(self, other):
    lvals = self._ndarray_values

    if self.is_monotonic and other.is_monotonic:
        rvals = other._ndarray_values
        return self._inner_indexer(lvals, rvals)[0]

    other_uniq = other.unique()
    indexer = other_uniq.get_indexer(lvals)
    indexer = indexer.take((indexer != -1).nonzero()[0])

    return other_uniq.take(indexer)._ndarray_values

def mixed(self, other):
    lvals = self._ndarray_values
    rvals = other._ndarray_values

    if self.is_monotonic and other.is_monotonic:
        return self._inner_indexer(lvals, rvals)[0]

    other_uniq = set(rvals)
    seen = set()
    return [x for x in lvals if x in other_uniq and not (x in seen or seen.add(x))]

size = 100000 
left = pd.MultiIndex.from_arrays([np.arange(1, size), np.arange(1, size)])
right = pd.MultiIndex.from_arrays([np.arange(1, size//10)[::-1], np.arange(1, size//10)[::-1]])
right_monotonic = pd.MultiIndex.from_arrays([np.arange(1, size//10), np.arange(1, size//10)])

for r in [right, right_monotonic]:
    print(f"\nBoth monotonic: {r.is_monotonic}\n---------------")
    expected = pd.MultiIndex.from_tuples(pr(left, r))
    actual_indexer = pd.MultiIndex.from_tuples(indexer(left, r))
    assert_index_equal(expected, actual_indexer)
    actual_mixed = pd.MultiIndex.from_tuples(mixed(left, r))
    assert_index_equal(expected, actual_mixed)

    print("master implementation:")
    %timeit -n 10 master(left, r)
    print("pr implementation:")
    %timeit -n 10 pr(left, r)
    print("Suggested indexer implementation:")
    %timeit -n 10 indexer(left, r)
    print("Mixed implementation:")
    %timeit -n 10 mixed(left, r)

Both monotonic: False
---------------
master implementation:
18.2 ms ± 5.36 ms per loop (mean ± std. dev. of 7 runs, 10 loops each)
pr implementation:
10.6 ms ± 625 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)
Suggested indexer implementation:
170 ms ± 4.36 ms per loop (mean ± std. dev. of 7 runs, 10 loops each)
Mixed implementation:
10.2 ms ± 562 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)

Both monotonic: True
---------------
master implementation:
15.1 ms ± 984 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)
pr implementation:
10.4 ms ± 730 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)
Suggested indexer implementation:
2.56 ms ± 292 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)
Mixed implementation:
2.63 ms ± 199 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)

'platform': 'Darwin',
'platform-release': '19.2.0',
'architecture': 'x86_64',
'ram': '16 GB'

[jreback]

looks good. can you add a whatsnew note in bugfixes for MI. ping on green.

[jreback]

pls merge master as well

[jeffzi]

@jreback All green :)

[jreback]

lgtm. @jbrockmendel

[jbrockmendel]

Couple of comments/questions, no deal-breakers. looks nice

[WillAyd]

Thanks @jeffzi great first PR