Don't ignore missing stubs in setuptools#10058

Avasam · 2023-04-17T21:53:37Z

Given the push for the deprecation of distutils, it'd be a good idea indeed to ensure that all stubs in setuptools are accounted for.

Fixed stubtest_allowlist entries
Removed ignore_missing_stub = true
Added missing __all__
Added new default values
Standardized imports (there was a mix of relative and absolute)

…ignore-missing-stubs-in-setuptools

stubs/setuptools/pkg_resources/__init__.pyi

JelleZijlstra · 2023-04-26T20:40:15Z

stubs/setuptools/setuptools/command/editable_wheel.pyi

+ def __enter__(self) -> None: ...
+ def __exit__(self, _exc_type, _exc_value, _traceback) -> None: ...


These are unannotated, but I don't think that should imply None since the body is just ....

Is it fine if all setuptool classes that implement the EditableStrategy protocol have the same return type for their methods? (None for __call__ and __exit__, Self for __enter__)

Maybe for __exit__, but it feels too restrictive for __enter__.

JelleZijlstra · 2023-04-26T20:40:43Z

stubs/setuptools/setuptools/command/editable_wheel.pyi

+ path_entries: Incomplete
+ def __init__(self, dist: Distribution, name: str, path_entries: list[Path]) -> None: ...
+ def __call__(self, wheel: _WheelFile, files: list[str], mapping: dict[str, str]): ...
+ def __enter__(self): ...


Suggested change
def__enter__(self): ...
def__enter__(self)->Self: ...
(Plus import)

JelleZijlstra · 2023-04-26T20:41:07Z

stubs/setuptools/setuptools/command/editable_wheel.pyi

+ build_lib: Incomplete
+ def __init__(self, dist: Distribution, name: str, auxiliary_dir: _Path, build_lib: _Path) -> None: ...
+ def __call__(self, wheel: _WheelFile, files: list[str], mapping: dict[str, str]): ...
+ def __enter__(self): ...


Suggested change
def__enter__(self): ...
def__enter__(self)->Self: ...

JelleZijlstra · 2023-04-26T21:12:00Z

stubs/setuptools/setuptools/depends.pyi

+
+__all__ = ["Require", "find_module", "get_module_constant", "extract_constant"]
+
+def find_module(module, paths=None) -> tuple[IO[Any], str, tuple[str, Literal["", "r", "rb"], Literal[7, 6, 1, 2, 3, -1]]]: ...


Suggested change
deffind_module(module, paths=None) ->tuple[IO[Any], str, tuple[str, Literal["", "r", "rb"], Literal[7, 6, 1, 2, 3, -1]]]: ...
deffind_module(module, paths=None) ->tuple[IO[Any], str|None, tuple[str, Literal["", "r", "rb"], Literal[7, 6, 1, 2, 3, -1]]]: ...
I see some paths where it's None.

AlexWaygood

I didn't finish looking at this, but here's a partial review I started -- mostly a few suggestions for some type annotations that look fairly obvious :)

AlexWaygood · 2023-04-17T23:22:11Z

stubs/setuptools/pkg_resources/__init__.pyi

+ requirements: Iterable[Requirement],
+ env: Environment | None = None,
+ installer: _InstallerType | None = None,
+ replace_conflicting=False,


Suggested change
replace_conflicting=False,
replace_conflicting: bool=False,

AlexWaygood · 2023-04-17T23:22:23Z

stubs/setuptools/pkg_resources/__init__.pyi

 ) -> list[Distribution]: ...
 def add(self, dist: Distribution, entry: str | None = None, insert: bool = True, replace: bool = False) -> None: ...
- def subscribe(self, callback: Callable[[Distribution], object]) -> None: ...
+ def subscribe(self, callback: Callable[[Distribution], object], existing=True) -> None: ...


Suggested change
defsubscribe(self, callback: Callable[[Distribution], object], existing=True) ->None: ...
defsubscribe(self, callback: Callable[[Distribution], object], existing: bool=True) ->None: ...

AlexWaygood · 2023-04-17T23:28:59Z

stubs/setuptools/pkg_resources/__init__.pyi

 # TODO: change this to packaging.markers.Marker | None once we can import
 # packaging.markers
 marker: Incomplete | None
+ def __init__(self, requirement_string) -> None: ...


It'll be the same as https://github.com/pypa/packaging/blob/55584fb5ca327ba38b74ca5c668125caaebd9a5d/src/packaging/requirements.py#L33
Suggested change
def__init__(self, requirement_string) ->None: ...
def__init__(self, requirement_string: str) ->None: ...

AlexWaygood · 2023-04-17T23:31:53Z

stubs/setuptools/pkg_resources/__init__.pyi

+ def __lt__(self, other) -> bool: ...
+ def __le__(self, other) -> bool: ...
+ def __gt__(self, other) -> bool: ...
+ def __ge__(self, other) -> bool: ...
+ def __eq__(self, other) -> bool: ...
+ def __ne__(self, other) -> bool: ...


Suggested change
def__lt__(self, other) ->bool: ...
def__le__(self, other) ->bool: ...
def__gt__(self, other) ->bool: ...
def__ge__(self, other) ->bool: ...
def__eq__(self, other) ->bool: ...
def__ne__(self, other) ->bool: ...
def__lt__(self, other: Distribution) ->bool: ...
def__le__(self, other: Distribution) ->bool: ...
def__gt__(self, other: Distribution) ->bool: ...
def__ge__(self, other: Distribution) ->bool: ...
def__eq__(self, other: object) ->bool: ...
def__ne__(self, other: object) ->bool: ...

AlexWaygood · 2023-04-17T23:32:17Z

stubs/setuptools/pkg_resources/__init__.pyi

 @classmethod
 def from_filename(cls, filename: str, metadata: _MetadataType = None, **kw: str | None | int) -> Distribution: ...
- def activate(self, path: list[str] | None = None) -> None: ...
+ def activate(self, path: list[str] | None = None, replace=False) -> None: ...


Suggested change
defactivate(self, path: list[str] |None=None, replace=False) ->None: ...
defactivate(self, path: list[str] |None=None, replace: bool=False) ->None: ...

AlexWaygood · 2023-04-17T23:33:55Z

stubs/setuptools/pkg_resources/__init__.pyi

-
-class IResourceManager:
+@type_check_only
+class IResourceManager(Protocol):


If this class doesn't exist at runtime, should it maybe be private?
Suggested change
classIResourceManager(Protocol):
class_IResourceManager(Protocol):

Isn't that what @type_check_only is for? Granted support is still limited in mypy (python/mypy#15146, python/mypy#9531)
The source docstrings do call it IResourceManager, so it's the right name, makes it easier to find and import, whilst being safer with checkers that support the annotation.

True, mypy has been pretty slow to implement this feature :)
Elsewhere in typeshed, though, we generally don't rely on a feature unless it's supported by all major type checkers. We use @type_check_only in a few places already, but those are basically all very special cases (builtins.pyi, typing.pyi) where our standard option of making the name private isn't available.

…ignore-missing-stubs-in-setuptools

Avasam · 2023-04-29T16:53:49Z

Oh neat, you can request review from more than one person now on GitHub.

Avasam · 2023-04-29T17:10:24Z

Pretty sure pyright is failing here because types-setuptools is installed (https://github.com/python/typeshed/actions/runs/4839817600/jobs/8625083762?pr=10058#step:4:55). (notice there's no error for FileMetadata, PathMetadata and EggMetadata). Same errors locally with types-setuptools installed.

AlexWaygood · 2023-05-01T18:11:07Z

Pretty sure pyright is failing here because types-setuptools is installed (https://github.com/python/typeshed/actions/runs/4839817600/jobs/8625083762?pr=10058#step:4:55). (notice there's no error for FileMetadata, PathMetadata and EggMetadata). Same errors locally with types-setuptools installed.

Seems your diagnosis was spot on!

github-actions · 2023-05-08T16:28:56Z

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

JelleZijlstra · 2023-05-19T00:02:39Z

Looks good, @AlexWaygood any further feedback?

@Avasam feel free to mark conversations as "resolved" if you have addressed them.

AlexWaygood

Haven't reviewed in depth, but all my previous points have been addressed, nothing stands out as weird from a skim-read, and I trust @JelleZijlstra to do a thorough review :)

AlexWaygood

Haven't reviewed in depth, but all my previous points have been addressed, nothing stands out as weird from a skim-read, and I trust @JelleZijlstra to do a thorough review :)

Avasam added 3 commits April 17, 2023 17:47

Don't ignore missing stubs in setuptools
ec253ca

Merge branch 'main' ofhttps://github.com/python/typeshedinto Don't-…
b9495d8
…ignore-missing-stubs-in-setuptools

remove version dependant default
e72ce69

This comment has been minimized.
Sign in to view

Avasam added 2 commits April 17, 2023 19:03

Fix pytype and linux errors
f05d076

Confirmed darwin is equivalent to linux
bdf18ca

This comment has been minimized.
Sign in to view

JelleZijlstra reviewed Apr 26, 2023
View reviewed changes

AlexWaygood reviewed Apr 26, 2023
View reviewed changes

Avasam added 2 commits April 28, 2023 00:11

Merge branch 'main' ofhttps://github.com/python/typeshedinto Don't-…
4057ff6
…ignore-missing-stubs-in-setuptools

Address PR comments + merge from main
d0485e8

This comment has been minimized.
Sign in to view

Try to add relevant tests
6dfcf40

Avasam requested review from AlexWaygood and JelleZijlstra April 29, 2023 16:53

This comment has been minimized.
Sign in to view

Avasam mentioned this pull request Apr 29, 2023
pyright gives types found in site-packages higher priority than types found in typeshed #10112
Closed

AlexWaygood mentioned this pull request Apr 30, 2023
Alternative fix for pyright CI issues #10121
Merged

Merge branch 'main' into Don't-ignore-missing-stubs-in-setuptools
4e0db81

This comment has been minimized.
Sign in to view

Merge branch 'main' into Don't-ignore-missing-stubs-in-setuptools
dc9ff1d

Merge branch 'main' into Don't-ignore-missing-stubs-in-setuptools
373dabb

JelleZijlstra approved these changes May 19, 2023
View reviewed changes

AlexWaygood approved these changes May 19, 2023
View reviewed changes

AlexWaygood merged commit b5c9d8d into python:mainMay 19, 2023

Avasam deleted the Don't-ignore-missing-stubs-in-setuptools branch November 28, 2023 20:17

Avasam mentioned this pull request Nov 28, 2023
Bump setuptools to 69.0.* #11069
Merged

		def __enter__(self) -> None: ...
		def __exit__(self, _exc_type, _exc_value, _traceback) -> None: ...


		__all__ = ["Require", "find_module", "get_module_constant", "extract_constant"]

		def find_module(module, paths=None) -> tuple[IO[Any], str, tuple[str, Literal["", "r", "rb"], Literal[7, 6, 1, 2, 3, -1]]]: ...

	deffind_module(module, paths=None) ->tuple[IO[Any], str, tuple[str, Literal["", "r", "rb"], Literal[7, 6, 1, 2, 3, -1]]]: ...
	deffind_module(module, paths=None) ->tuple[IO[Any], str\|None, tuple[str, Literal["", "r", "rb"], Literal[7, 6, 1, 2, 3, -1]]]: ...

	defsubscribe(self, callback: Callable[[Distribution], object], existing=True) ->None: ...
	defsubscribe(self, callback: Callable[[Distribution], object], existing: bool=True) ->None: ...

	def__init__(self, requirement_string) ->None: ...
	def__init__(self, requirement_string: str) ->None: ...

-def__lt__(self, other) ->bool: ...
-def__le__(self, other) ->bool: ...
-def__gt__(self, other) ->bool: ...
-def__ge__(self, other) ->bool: ...
-def__eq__(self, other) ->bool: ...
-def__ne__(self, other) ->bool: ...
+def__lt__(self, other: Distribution) ->bool: ...
+def__le__(self, other: Distribution) ->bool: ...
+def__gt__(self, other: Distribution) ->bool: ...
+def__ge__(self, other: Distribution) ->bool: ...
+def__eq__(self, other: object) ->bool: ...
+def__ne__(self, other: object) ->bool: ...

	defactivate(self, path: list[str] \|None=None, replace=False) ->None: ...
	defactivate(self, path: list[str] \|None=None, replace: bool=False) ->None: ...

	classIResourceManager(Protocol):
	class_IResourceManager(Protocol):

Uh oh!

Don't ignore missing stubs in setuptools#10058

Don't ignore missing stubs in setuptools #10058

Uh oh!

Conversation

Avasam commented Apr 17, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AvasamApr 29, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexWaygood left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AvasamApr 28, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

Avasam commented Apr 29, 2023

Uh oh!

This comment has been minimized.

Avasam commented Apr 29, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlexWaygood commented May 1, 2023

Uh oh!

This comment has been minimized.

github-actionsbot commented May 8, 2023

Uh oh!

JelleZijlstra commented May 19, 2023

Uh oh!

AlexWaygood left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexWaygood left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Avasam commented Apr 17, 2023•
edited
Loading

AvasamApr 29, 2023•
edited
Loading

AlexWaygood left a comment •
edited
Loading

AvasamApr 28, 2023•
edited
Loading

Avasam commented Apr 29, 2023•
edited
Loading

AlexWaygood left a comment •
edited
Loading

AlexWaygood left a comment •
edited
Loading