Skip to content

Add stubs for pynput package #7177

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

Merged
merged 5 commits into from
Feb 23, 2022
Merged

Add stubs for pynput package #7177

merged 5 commits into from
Feb 23, 2022

Conversation

faresbakhit
Copy link
Contributor

Fixes #4328

Comment on lines 1 to 9
pynput
pynput._info
pynput._util.RESOLUTIONS
pynput.keyboard
pynput.keyboard._base
pynput.keyboard._dummy
pynput.mouse
pynput.mouse._base
pynput.mouse._dummy
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are all of those on the stubtest allowlist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

stubtest fails to import pynput since there's no display server running (e.g. x11)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see. We should probably re-introduce the flag to disable stubtest in METADATA.toml we had for a short time. I will send a PR tomorrow.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

Am I missing something? stubtest does not appear to fail to import pynput in CI. In fact, if it was failing, you wouldn't see any errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hauntsaninja Here is the failing CI test in the first commit:

https://github.com/python/typeshed/runs/5163416378

error: pynput failed to import: this platform is not supported: ('failed to acquire X connection: Bad display name ""', DisplayNameError(''))

I added the allowlist in the commit after it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, I see how I confused myself. Sorry about that, thanks!

@srittau
Copy link
Collaborator

srittau commented Feb 21, 2022

@rchen152 Could you have a look at the pytype failure? I don't see anything obvious wrong.

@srittau
Copy link
Collaborator

srittau commented Feb 22, 2022

Depends on #7351. This adds support for the stubtest = false key back to METADATA.toml.

@rchen152
Copy link
Collaborator

Looking into the pytype error.

@rchen152
Copy link
Collaborator

This change seems to get rid of the pytype_test failure:

--- a/stubs/pynput/pynput/keyboard/_base.pyi
+++ b/stubs/pynput/pynput/keyboard/_base.pyi
@@ -101,8 +101,7 @@ class Controller:
     def pressed(self, *args: str | Key | KeyCode) -> Iterator[None]: ...
     def type(self, string: str) -> None: ...
     @property  # type: ignore[misc]
-    @contextlib.contextmanager
-    def modifiers(self) -> Iterator[set[Key]]: ...
+    def modifiers(self) -> contextlib.AbstractContextManager[Iterator[set[Key]]]: ...
     @property
     def alt_pressed(self) -> bool: ...
     @property

@faresbakhit
Copy link
Contributor Author

Thanks! Just added the change in the last commit. 😄

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me, although I didn't compare it to the implementation. A few nits below.

Co-authored-by: Sebastian Rittau <[email protected]>
@srittau srittau merged commit b55fed4 into python:master Feb 23, 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.

pynput
5 participants