Skip to content
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

Added PyInstaller stubs for all documented modules & packages #8702

Merged
merged 19 commits into from
Sep 15, 2022

Conversation

Avasam
Copy link
Collaborator

@Avasam Avasam commented Sep 7, 2022

stubgen was timing out (python/mypy#9103), so I had to generate these stubs using Pyright rather than create_baseline_stubs.py. Hopefully I didn't miss anything.

ignore_missing_stub set to true because most modules are not meant to be used, yet are not marked as private.
Documented modules:

pyi_splash is a fake module that only exists once the app is frozen. It is meant to manipulate the splash screen.
image

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Comment on lines 12 to 28
def __init__(
self,
scripts: Iterable[StrOrBytesPath],
pathex=...,
binaries=...,
datas=...,
hiddenimports=...,
hookspath=...,
hooksconfig: dict[str, dict[str, object]] | None = ...,
excludes=...,
runtime_hooks=...,
cipher=...,
win_no_prefer_redirects: bool = ...,
win_private_assemblies: bool = ...,
noarchive: bool = ...,
module_collection_mode=...,
) -> None: ...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused about why pyright isn't emitting an error here. There's no entry in the pyrightconfig.stricter.json exclude-list, so it should be erroring for any parameters that don't have a type annotation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a guess:
pyrightconfig.stricter.json has "typeCheckingMode": "basic"
and doesn't configure reportMissingParameterType. The default value for that setting being None

Copy link
Member

@AlexWaygood AlexWaygood Sep 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea -- anyway, it's not your fault, and we certainly don't have a requirement that all parameters need to be annotated when first adding stubs :)

I'll look into it and try to work it out...

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Mostly looks good, but a few points below :) all are fairly minor!

My main concern is using LiteralString for parameter annotations, which seems quite restrictive. Not sure about that.

stubs/pyinstaller/PyInstaller/building/build_main.pyi Outdated Show resolved Hide resolved
stubs/pyinstaller/PyInstaller/compat.pyi Outdated Show resolved Hide resolved
stubs/pyinstaller/PyInstaller/compat.pyi Outdated Show resolved Hide resolved
stubs/pyinstaller/PyInstaller/compat.pyi Outdated Show resolved Hide resolved
stubs/pyinstaller/PyInstaller/utils/hooks/__init__.pyi Outdated Show resolved Hide resolved
stubs/pyinstaller/PyInstaller/utils/hooks/conda.pyi Outdated Show resolved Hide resolved
stubs/pyinstaller/PyInstaller/utils/hooks/conda.pyi Outdated Show resolved Hide resolved
stubs/pyinstaller/PyInstaller/utils/hooks/conda.pyi Outdated Show resolved Hide resolved
stubs/pyinstaller/PyInstaller/utils/hooks/conda.pyi Outdated Show resolved Hide resolved
@Avasam
Copy link
Collaborator Author

Avasam commented Sep 15, 2022

Here's a batch of fixes from your comments, and a few discussion points.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just needs a couple more allowlist entries. I'll apply them and then merge.

@github-actions

This comment has been minimized.

PyInstaller.utils.tests
PyInstaller.utils.win32.*
# Explicitly private implementation details
PyInstaller\._.*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that this allowlist entry ends up looking like an emoji

@github-actions

This comment has been minimized.

1 similar comment
@github-actions
Copy link
Contributor

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

@AlexWaygood AlexWaygood merged commit c9346f3 into python:master Sep 15, 2022
@AlexWaygood
Copy link
Member

Thanks @Avasam! :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants