Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 2k
disallow subclassing any#9491
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
disallow subclassing any #9491
Uh oh!
There was an error while loading. Please reload this page.
Conversation
Avasam commented Jan 11, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Avasam commented Jan 11, 2023
|
Avasam commented Jan 11, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
|
AlexWaygood commented Jan 11, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Those parts of the |
Avasam commented Jan 11, 2023
I haven't looked at tqdm in details yet, but it's possible the type are quite simple (seeing how they seem to be some kind of callback interface). We may be able to do without depending on any external stub. |
This comment has been minimized.
This comment has been minimized.
AlexWaygood commented Jan 11, 2023
Is there an equivalent pyright setting that we should also enable? |
Avasam commented Jan 12, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Pyright has |
…ypeshed into disallow-subclassing-any
…ow-subclassing-any
Avasam commented Jan 12, 2023
Not sure how I can fix this one in stdlib/distutils
|
JelleZijlstra commented Jan 12, 2023
Right, that is a part of the stdlib that depends on a third-party package (docutils). Given that distutils is deprecated anyway, I would just type ignore that one and not spend more time on it. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
AlexWaygood commented Jan 13, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
One thing I worry about a little bit here: some of these have been pretty difficult to fix, and some others have been easy but non-obvious (e.g. the fix for sqlalchemy was just to delete a bunch of code 😉). The audit you've been doing here has been really great for improving the quality of typeshed's stubs, so whatever we decide here, you've done some fantastic work! But if we disallow subclassing I'm undecided on this point — very interested in hearing other people's thoughts! In general, I'm extremely on board with making our tests stricter wherever possible, so I'm somewhat torn :) |
Avasam commented Jan 13, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
My thoughts: _BaseClass: TypeAlias=Incomplete# Explicitly marked as incomplete# Some comment explaining whyclassSomeClass(_BaseClass): ... # type: ignore[misc]Which is exactly what happened in pillow. Now you'd at least get the warning from mypy. Which is helpful. |
AlexWaygood commented Jan 13, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Yup. But my worry is that if we say "just So are there any cases where having mypy emit the error would have changed the way we would have approached writing the stub in the first instance? I'm not sure. |
This comment has been minimized.
This comment has been minimized.
Avasam commented Feb 1, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
For what its worth: the regression tests do fail on sublcassing any. I had to There are still the following that would need to be completed first: |
This comment has been minimized.
This comment has been minimized.
srittau commented Feb 1, 2023
I share @AlexWaygood's concerns. There are valid reason to subclass |
This comment has been minimized.
This comment has been minimized.
…ow-subclassing-any
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
Until they're all fixed. This serves as a tracker. (look at the CI results)
Quick link to latest CI result [2020/01/14]: https://github.com/python/typeshed/actions/runs/3918898421/jobs/6699566039