TYP: get_indexer

[jbrockmendel]

@simonjayhawkins im at a loss on why im getting a mypy complaint:

error: Overloaded function signatures 1 and 2 overlap with incompatible return types  [misc]

AFAICT they should be compatible. Am I missing something obvious?

[jbrockmendel]

@simonjayhawkins gentle ping

[jreback]

can you rebase

[jbrockmendel]

cc @WillAyd any idea whats going on here? my confusion only deepens when i try to do the same thing for Index.join

[jbrockmendel]

rebased, still no idea whats going on with mypy

[jbrockmendel]

@MarcoGorelli #40197 makes me think you have a better grasp of @overload than i do. can you tell what im doing wrong here?

[MarcoGorelli]

Hey @jbrockmendel ,

Yeah, I'm embarassed by how long I spent making sense of this in order to do #40200 and #40197 (maybe I should write a blog post* about it)

I explain it a bit more clearly in ##40200 (comment) , but in essence, you need an overload for each combination of keyword argument which precedes the one you're overloading

Here, there are no keyword arguments preceding unique, so you'll just need overloads for:

• Literal[True] = ...
• Literal[False]
• bool = ...

What's confusing here is that the bool overload is needed - IMO, it shouldn't be, as the union of Literal[True] and Literal[False] should be regarded as the same as bool, but I opened an issue about that in mypy and have had no response: python/mypy#10194

---

*just put something down while it's still fresh in my mind - and in case it's useful to you: https://medium.com/@m.e.gorelli/making-sense-of-typing-overload-437e6deecade

[MarcoGorelli]

Indeed, this seems to work:

    @overload
    def _get_indexer_non_comparable(
        self, target: Index, method, unique: Literal[True] = ...
    ) -> np.ndarray:
        # returned ndarray is np.intp
        ...

    @overload
    def _get_indexer_non_comparable(
        self, target: Index, method, unique: Literal[False]
    ) -> tuple[np.ndarray, np.ndarray]:
        # both returned ndarrays are np.intp
        ...

    @overload
    def _get_indexer_non_comparable(
        self, target: Index, method, unique: bool = ...
    ) -> Union[tuple[np.ndarray, np.ndarray], np.ndarray]:
        ...

[jbrockmendel]

Indeed, this seems to work:

It does, but AFAICT it isnt the 3rd @overload that is making a difference, its the fact that in the second one you wrote unique: Literal[False] instead of unique: Literal[False] = ...

[jbrockmendel]

It does, but AFAICT it isnt the 3rd @overload that is making a difference, its the fact that in the second one you wrote unique: Literal[False] instead of unique: Literal[False] = ...

related: what if the argument for which we need to omit the = ... comes after another keyword-only argument?

update: it looks like just pretending in the overloading line that that previous argument is not keyword-only works. but that feels very sketchy

[MarcoGorelli]

It does, but AFAICT it isnt the 3rd @overload that is making a difference, its the fact that in the second one you wrote unique: Literal[False] instead of unique: Literal[False] = ...

You need to omit =... for Literal[False] because the default is Literal[True].

Otherwise, consider what happens if you don't do _get_indexer_non_comparable(target, method). Which overload should mypy use? Literal[False]=... and Literal[True]=... both match, and their return types are incompatible, so mypy throws an error. On the other hand, if you type the overloads as Literal[True]=... and Literal[False], then if you don't specify the value of inplace, then mypy will know to use the one which accepts defaults, i.e. the Literal[True] overload.

Check the mypy docs on this:

mypy will detect and prohibit inherently unsafely overlapping overloads on a best-effort basis.
Two variants are considered unsafely overlapping when both of the following are true:
    - All of the arguments of the first variant are compatible with the second.
    - The return type of the first variant is not compatible with (e.g. is not a subtype of) the second.

related: what if the argument for which we need to omit the = ... comes after another keyword-only argument?

See what I did for set_axis - you need to provide overloads (without =...) for each combination of keyword arguments which comes before the one you're overloading

---

In case you haven't already, see this open issue in mypy about this topic: python/mypy#6580

[jbrockmendel]

Are any of the remaining comments actionable?

[jbrockmendel]

gentle ping @MarcoGorelli

[MarcoGorelli]

As far as I can tell, this is correct (though probably I don't know this part of the codebase well enough to merge)

---

The only part here I'm not totally comfortable commenting on is the change from missing to ensure_platform_int(missing), as that involves making sense of pxi.in files, which I've not (yet) done. If that change is correct, then I'm pretty sure the rest is fine

[MarcoGorelli]

perhaps you could just comment on why this change is necessary, and what it does?

[jbrockmendel]

i think this was from before a separate PR ensured that self._engine.get_indexer_non_unique already returned ndarray[intp], but this makes it a little bit more explicit (and is cheap)

[MarcoGorelli]

Thanks for explaining - thanks @jbrockmendel