-
-
Notifications
You must be signed in to change notification settings - Fork 367
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
Fix type errors in Pyright #999
Conversation
MyPy treats the user-declared `MYPY` constant equivalently to `typing.TYPE_CHECKING` (see https://mypy.readthedocs.io/en/stable/more_types.html#conditional-overloads). This can be used to trick other type checkers into ignoring MyPy-specific types. `AttrsInstance` needs to be declared in a separate stub file so as to not export `MYPY`.
`x as y` only works if x and y are the same.
Hm is there a way to get rid of the namespace? Otherwise you'll have to do it inline I'm afraid |
The problem with that is that Pyright solves |
Maybe I was unclear. What I meant is putting the logic into |
I created a new file because the |
We can keep just from ._typing_compat import MYPY
if MYPY:
class AttrsInstance(Protocol):
__attrs_attrs__: ClassVar[Any]
else:
AttrsInstance = Any
# Pyright ignores the else cond
if MYPY:
class AttrsInstance(Protocol):
__attrs_attrs__: ClassVar[Any]
else:
AttrsInstance: Any
# Pyright reports that 'Class declaration "AttrsInstance" is obscured by a declaration of the same name'
# for `AttrsInstance: Any`
class AttrsInstance_(Protocol):
__attrs_attrs__: ClassVar[Any]
if MYPY:
AttrsInstance = AttrsInstance_
else:
AttrsInstance = Any
# No errors in either type checker but mypy displays the original name with the underscore: `type[AttrsInstance_]`
# which is probably no better than having it under a different ns |
I can live with MYPY in our stubs if that's all that's stopping you? Sorry on the go and want to unblock you, but can't peruse in detail |
Okay, there are several issues here. The first one is not wanting to expose The second issue (the thorniest of them all) is conditionally declaring a type. You can typically only do this using a small set of constants. Thinking about this some more, that the if MYPY:
# A protocol to be able to statically accept an attrs class.
class AttrsInstance_(Protocol):
__attrs_attrs__: ClassVar[Any]
else:
class AttrsInstance_(Protocol):
pass This will produce a localised type error in Pyright about The third issue is retaining the type's original qualified name ( class AttrsInstance(AttrsInstance_, Protocol):
pass |
So, uh. I think this protocol will need to be importable, so people can mix it in. So it'll need to be in a .py file instead. Does that complicate things? |
It shouldn't matter. |
Co-authored-by: layday <layday@protonmail.com>
just a heads up: i've stolen the protection logic because i had to protect the pyright tests against 3.7 anyways. i've credited you, but sorry for the conflicts |
That's no problem at all :) Eric Traut has commented on the implementation here at microsoft/pyright#3808 (reply in thread). |
Any thoughts on how to proceed here? I would like to change the chromatic composition of my CI eventually 😛 |
It looks good enough to my ignorant eyes – @Tinche? |
This looks good to me, but I'd really like to have the protocol importable by normal code. @layday can you refactor this logic out of .pyi files? |
Do you still want it to be defined in |
Bear in mind that
|
I tried installing this into our of our services and it seems to work great! |
I guess the current implementation is going with #3, which seems fine to me. |
@layday Add a town crier fragment please and we're good to merge. |
Thanks. @hynek this can go in I think. |
Thanks everyone! |
Summary
MyPy treats the user-declared
MYPY
constant equivalentlyto
typing.TYPE_CHECKING
(see https://mypy.readthedocs.io/en/stable/more_types.html#conditional-overloads).This can be used to trick other type checkers into ignoring
MyPy-specific types.
AttrsInstance
needs to be declared in a separate stub file so as tonot export
MYPY
.Pull Request Check List
Our CI fails if coverage is not 100%.
.pyi
).tests/typing_example.py
.attr/__init__.pyi
, they've also been re-imported inattrs/__init__.pyi
.docs/api.rst
by hand.@attr.s()
have to be added by hand too.versionadded
,versionchanged
, ordeprecated
directives.Find the appropriate next version in our
__init__.py
file..rst
files is written using semantic newlines.changelog.d
.