REF: check monotonicity inside _can_use_libjoin

[jbrockmendel]

• closes #xxxx (Replace xxxx with the GitHub issue number)
• Tests added and passed if fixing a bug or adding a new feature
• All code checks passed.
• Added type annotations to new arguments/methods/functions.
• Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

[lukemanley]

I think the order here can have a difference. is_monotonic_increasing will provide is_unique "for free" if the index is both monotonic and unique and it hasn't already been cached.

import pandas as pd
import numpy as np

arr = np.arange(1_000_000)

%timeit idx=pd.Index(arr); idx.is_unique and idx.is_monotonic_increasing
# 68.7 ms ± 5.13 ms per loop (mean ± std. dev. of 7 runs, 10 loops each)

%timeit idx=pd.Index(arr); idx.is_monotonic_increasing and idx.is_unique
# 2.91 ms ± 121 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

[jbrockmendel]

Good catch. i had expected the cache for all of these to get populated at the same time inside IndexEngine, will take a closer look

[jbrockmendel]

OK, looks like engine.is_monotonic_increasing will populate the engine.is_unique cache but not vice-versa. All of the paths within the engine that check is_unique do that check after checking self.is_monotonic_increasing. im inclined to update the engine code so that the unique check always populates the is_monotonic_increasing cache in order to 1) make the perf not dependent on the order these are accessed and 2) avoid populating engine.mapping in these cases (which allocates memory)

[lukemanley]

im inclined to update the engine code so that the unique check always populates the is_monotonic_increasing cache in order to 1) make the perf not dependent on the order these are accessed and 2) avoid populating engine.mapping in these cases (which allocates memory)

that sounds good to me

[jbrockmendel]

@lukemanley any interest in taking over on this one? im at risk of letting it fall through the cracks

[lukemanley]

@lukemanley any interest in taking over on this one? im at risk of letting it fall through the cracks

Sure, I'll take this one.

[lukemanley]

It looks like this is now down to one test failure: test_setitem_mix_of_nan_and_interval

The underlying issue currently exists on main and is similar to #54847.

Now that Index.is_monotonic_increasing gets called as part of Index.is_unique we're now hitting this behavior in the test:

import pandas as pd
from decimal import Decimal

idx = pd.Index([Decimal("NaN"), Decimal("NaN")])

# raises InvalidOperation: [<class 'decimal.InvalidOperation'>]
idx.is_monotonic_increasing

We could either check for nulls in algos.is_monotonic or put a try/except around Index.is_unique to catch this case. Kind of annoying either way it seems. Any other options?

[jbrockmendel]

or put a try/except around Index.is_unique to catch this case

Where would this live? some places would be uglier than others.

[lukemanley]

added to catch the Decimal("NaN") < Decimal("NaN") case

[lukemanley]

Where would this live? some places would be uglier than others.

Added to the existing try/except in IndexEngine._do_monotonic_check

This is now passing all tests.

[github-actions]

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

[lukemanley]

gentle ping @jbrockmendel if you have any thoughts on this

[jbrockmendel]

LGTM. Is the perf issue you noted addressed? One more merge main and ping on green pls

[lukemanley]

@jbrockmendel - merged main and now green. I think this is ready

[mroeschke]

Thanks @jbrockmendel

[jbrockmendel]

thanks @lukemanley