-
Notifications
You must be signed in to change notification settings - Fork 2.4k
refactor: use typing_extensions.Self
#9251
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
Conversation
typing_extensions.Selftyping_extensions.Self
citation required
if poetry imports from typing-extensions then it should express that dependency directly. |
Sure:
Yeah, I had been aware of that and happened to mention it, but I see it as irrelevant and personally think it shouldn't be relied on. AFAIK, it's not the direct reason
First things first, if we rely on pyright additionally to mypy, I believe it should be included in the CI suite. Probably some issues that pyright would typically report (and mypy wouldn't due to some of their discrepancies) could have been (or can in the future be) merged undetected.
Well, that's weird since the |
no part of what you quote says "therefore you do not need Most of the rest of what you write confuses stubs for typing-extensions with the typing-extensions package, I think it is - for the moment, just about - true that it would be sufficient to put
why even worry about adding the dependency? |
It's a conclusion. It just isn't explicitly stated in the docs.
Sorry, but I'm afraid you've completely missed my point.
When
Because it is unnecessary. Poetry already relies on a lot of packages. If you're still hesitant that installing I hope that that is convincing enough, if I myself hadn't been. |
|
as we have established, actual typecheckers do not approve of this approach: As I say, declaring the dependency only in the typing dependency group would I think be sufficient, though I still do not like it (for the reasons I already gave). CI might or might not catch missing dependencies. Eg it is very plausible to fail to spot a missing dependency on a package that is made transitively available by a test dependency. |
I don't think that warning would impact users of poetry? But yeah, it's true that that's a lint that pyright has and mypy doesn't
FWIW, while @bswck is correct that it is strictly speaking unnecessary to list Anyway, I think both ways of doing it are workable, and it's up to you folks to weigh the tradeoff appropriately :-) ping me again if you need me! |
|
pyright lint seems to have been in response to a bug report saying "you are failing to detect that typing-extensions is missing" - microsoft/pyright#7318 possibly that fix was an overshoot and they could be convinced that the lint is only desirable for imports made outside of |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |

Used
typing_extensions.Selfinstead of(self: T, ...) -> T.typing_extensionshas a special stub in typeshed and is seen by mypy even if not installed as a direct/transitive dependency, so there's no need to change anything in dependency specs.typing_extensionswas used instead oftypingbecause theSelftype was introduced in 3.11 with PEP 673 andtyping_extensionsoffers a backport for 3.8+, which is our case.Operation.skip()andOperation.unskip()have an anti-pattern, i.e. they returnSelfdespite operating in-place.This will be addressed in a separate PR; I need an idea how to address the API incompatibility that would bring.