Skip to content

Fix performance regression on numerical libraries - #22030

Open
ilevkivskyi wants to merge 4 commits into
python:masterfrom
ilevkivskyi:fix-perf-num
Open

ilevkivskyi wants to merge 4 commits into
python:masterfrom
ilevkivskyi:fix-perf-num

Conversation

@ilevkivskyi

Copy link
Copy Markdown
Member

My recent PR #22007, that improved overload vs callable subtyping, introduced ~10% performance regression on pandas/numpy/etc. There is no point in simplifying unions in code that is not visible to the user. Also m^2 / 2 + n^2 / 2 >= mn for all m and n, so IIUC this should be strictly better now.

cc @JukkaL

@github-actions

This comment has been minimized.

@ilevkivskyi

Copy link
Copy Markdown
Member Author

OK, the crash is good, it uncovers a flaw in callable unioning logic: We should not combine callables with unpacked kwargs, since although they look like they have same shape, they actually don't. I will hopefully fix later today or tomorrow.

@github-actions

This comment has been minimized.

@JukkaL

JukkaL commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator

This looks fine, but the original PR seems to have also regressed this (false negative):

from typing import Protocol, overload

class P(Protocol):
    def __call__(self, x: int | str) -> int | str: ...

@overload
def f(x: int) -> int: ...
@overload
def f(y: str) -> str: ...

p: P = f      # accepted, but shouldn't be
p(x="s")      # fails at runtime

@ilevkivskyi

Copy link
Copy Markdown
Member Author

This looks fine, but the original PR seems to have also regressed this (false negative)

This was semi-intentional. This kind of subtyping usually appears in the context of method overrides (not in context of callback passing) were we currently use ignore_pos_arg_names=True, so rejecting it completely would be inconsistent with callable vs callable method overrides. That said however, it looks like it should be not hard to simply pass the ignore_pos_arg_names value here, I will do this.

@ilevkivskyi

Copy link
Copy Markdown
Member Author

It looks like GitLab has some kind of outage

Cloning into 'cki-lib'...
fatal: unable to access 'https://gitlab.com/cki-project/cki-lib/': The requested URL returned error: 502

and consequently mypy_primer is broken. @hauntsaninja just FYI.

@github-actions

Copy link
Copy Markdown
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

scikit-learn (https://github.com/scikit-learn/scikit-learn)
- sklearn/externals/array_api_extra/testing/_testing.py:389: error: Unused "type: ignore" comment  [unused-ignore]

@ilevkivskyi

Copy link
Copy Markdown
Member Author

The primer is good, it was a false negative (different positional argument name were ignored).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants