Skip to content

Conversation

@Avasam
Copy link
Collaborator

@Avasam Avasam commented Sep 19, 2022

This was noticed in #8702 (comment)
Completes the types in keyboard and PyInstaller (which I've both added recently) so that this option can be turned on.

@github-actions

This comment has been minimized.

replace_paths=...,
implies=...,
graph: Incomplete | None = ...,
user_hook_dirs: Iterable[StrOrBytesPath] = ...,
Copy link
Member

Choose a reason for hiding this comment

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

Bytes paths don't work: PyInstaller.depend.analysis.PyiModuleGraph("x", [b"y"]) throws a TypeError.

Copy link
Collaborator Author

@Avasam Avasam Sep 21, 2022

Choose a reason for hiding this comment

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

TypeError: Can't mix strings and bytes in path components

This is probably the case in other parts of the stubs as well 😓 I'll go back through other StrOrBytesPath and make a PR to fix them when they're explicitly used with strings.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

graph: Incomplete | None = ...,
user_hook_dirs: Iterable[StrOrBytesPath] = ...,
excludes: Iterable[str] = ...,
path: Iterable[str] | None = ...,
Copy link
Member

Choose a reason for hiding this comment

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

These and further args are keyword-only, since they're forwarded through **kwargs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Didn't know keyword-only arguments were a thing. Thanks!

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Looks good now, thanks!

Your other PR introduced a few conflicts, I hope I got the merge right.

@github-actions
Copy link
Contributor

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

@JelleZijlstra JelleZijlstra merged commit dba47e0 into python:master Sep 21, 2022
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.

2 participants