fix: fill out more of distutils & setuptools dist #9895
Conversation
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
|
Is it a good idea to include something in a stub if it's identical type-wise to the base class but it does exist in the subclass? That is: class A:
def f(self):
return None
class B(A):
def f(self):
return super().f()Should that be: class A:
def f(self) -> None: ...
class B(A):
def f(self) -> None: ...Or class A:
def f(self) -> None: ...
class B(A): ...? This happens quite a bit in setuptool's Distribution. I can push the fix for the mistake but this would help in selecting the right fix - repeating the correct type hint or removing the repeated type hints. |
We generally omit methods on the subclass if they have identical signatures to the methods on the superclass. But there are a few exceptions (mostly for dunders) — e.g. |
|
But if the signature differs on the subclass, you should definitely include it. If it's incompatible with the superclass, you should add a Our job is to faithfully represent what happens at runtime. |
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
|
Is the stubtest taking into account side effects of running I can pull this out to a separate PR. |
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
|
Stubtest works by importing the module at runtime and dynamically inspecting the objects it finds in it, comparing the results to what it sees in the stub. So it often struggles with methods generated inside If need be, we will just have to add allowlist entries to tell it to be quiet about these methods. (Note: I haven't studied your proposed changes here yet; this is just a direct response to the question you posed above :) |
|
Okay, that would explain it. I'd have expected something simple like assigning inside an init method to also have a problem (thought that's not usually making a method, perhaps it's only checking methods?) Anyway, I've pulled that out separately, as I think it's easier to review on it's own. |
Yup, 99% of methods are statically defined, so if there's a method in the stub that it can't see a method at runtime, stubtest assumes you've made a mistake and emits an error. But 99% of instance attributes are assigned in Stubtest is basically just a series of hardcoded consistency checks; it doesn't have a generalised way of comparing the runtime to the stub. But it's very effective at what it does (it just has a few edge cases, and you've stumbled on one of them!). |
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
This comment has been minimized.
This comment has been minimized.
Is the there a way to silence that? Or should I just use |
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com> refactor: use overload for reinit command Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
|
Diff from mypy_primer, showing the effect of this PR on open source code: sphinx (https://github.com/sphinx-doc/sphinx)
- sphinx/setup_command.py: note: In member "initialize_options" of class "BuildDoc":
- sphinx/setup_command.py:107:26: error: "Distribution" has no attribute "verbose" [attr-defined]
isort (https://github.com/pycqa/isort)
- isort/setuptools_commands.py:34: error: "Distribution" has no attribute "packages" [attr-defined]
- isort/setuptools_commands.py:35: error: "Distribution" has no attribute "package_dir" [attr-defined]
- isort/setuptools_commands.py:36: error: "Distribution" has no attribute "packages" [attr-defined]
- isort/setuptools_commands.py:44: error: "Distribution" has no attribute "py_modules" [attr-defined]
- isort/setuptools_commands.py:45: error: "Distribution" has no attribute "py_modules" [attr-defined]
|
This was an attempt to fill out more of the typing for dist. I've re-synced
setuptools/_distutils/dist.pywithdistutils/dist.py, as the vendored version was missing a lot of info from the other one.