Skip to content

calling .register on typing.KeysView subclass causes false positive error Member "register" is unknown #9296

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.

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

Closed
jab opened this issue Nov 29, 2022 · 5 comments

Comments

@jab
Copy link
Contributor

jab commented Nov 29, 2022

Type checkers that include the recent changes from #8977 #9058 (e.g. pyright v1.1.281) now incorrectly emit an error like Member "register" is unknown for the following line of code:

https://github.com/jab/bidict/blob/522c9b69249d66c201fed608f95a5da8d3064ed8/bidict/_base.py#L51

Type checkers that do not include those changes (e.g. pyright v1.1.278) do not emit any error for that line, as expected.

Copy/pasted excerpt:

import typing as t

from ._typing import KT  # a typevar


class BidictKeysView(t.KeysView[KT], t.ValuesView[KT]):
    """Since the keys of a bidict are the values of its inverse (and vice versa),
    the :class:`~collections.abc.ValuesView` result of calling *bi.values()*
    is also a :class:`~collections.abc.KeysView` of *bi.inverse*.
    """


dict_keys: t.Type[t.KeysView[t.Any]] = type({}.keys())
BidictKeysView.register(dict_keys)  # <-- false positive error emitted here

Please let me know if there is any other info I can provide. Thank you!

@JelleZijlstra
Copy link
Member

I don't think the typeshed change you link to is relevant. Probably #9058 is the culprit instead.

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Nov 29, 2022

It's relevant in that #9058 wouldn't have happened if not for #8977.

Also, #8977 was mostly done for aesthetic reasons, so if we need a short-term fix in typeshed, I suggest we roll back #8977 (as per option 2 in #8977 (comment))

Of course, in the medium term we should fix the mypy bug this exposed #9058 (comment) / python/mypy#13986 (maybe it's already fixed?). The reason CI didn't complain is because this needs python/mypy#13579 to break, which was not part of a released mypy at the time.

I'm assuming @jab ran into this in the context of bidict. We do have bidict in mypy_primer and didn't see any regressions, so maybe this is something mypy has some special casing for? There's a chance this might be the very first time we've had an issue where a pyright_primer would be useful

@AlexWaygood
Copy link
Member

Also, #8977 was mostly done for aesthetic reasons, so if we need a short-term fix in typeshed, I suggest we roll back #8977 (as per option 2 in #8977 (comment))

Agreed that rolling back #8977 and #9058 seems like a good option for now.

@jab
Copy link
Contributor Author

jab commented Dec 10, 2022

Thanks @JelleZijlstra, @hauntsaninja, and @AlexWaygood for looking at this.

Agreed that rolling back #8977 and #9058 seems like a good option for now.

I'm new to this codebase, but submitted #9348 with these reversions, along with a regression test. Hope this helps.

I'm assuming @jab ran into this in the context of bidict.

Yes.

We do have bidict in mypy_primer

TIL, cool and thank you!

and didn't see any regressions, so maybe this is something mypy has some special casing for?

Do you know, @JelleZijlstra?

There's a chance this might be the very first time we've had an issue where a pyright_primer would be useful

FWIW, I believe this is not the very first (and maybe not even the second) time.

Looks like there's already a dedicated issue for considering a pyright_primer (#7642), maybe now the time is right?

Thanks again for working together on this.

@JelleZijlstra
Copy link
Member

and didn't see any regressions, so maybe this is something mypy has some special casing for?

Do you know, @JelleZijlstra?

I haven't looked but this feels like an area where typecheckers need to hardcode a lot of behavior, since Protocol classes and ABCs are special in ways that are core of the type system. It's not surprising that different type checkers handle this case differently.

Closing as #9348 was merged.

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

No branches or pull requests

4 participants