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 pyautogui stubs #8654

Merged
merged 16 commits into from
Sep 3, 2022
Merged

Added pyautogui stubs #8654

merged 16 commits into from
Sep 3, 2022

Conversation

Avasam
Copy link
Sponsor Collaborator

@Avasam Avasam commented Aug 31, 2022

No description provided.

@github-actions

This comment has been minimized.

@Avasam
Copy link
Sponsor Collaborator Author

Avasam commented Aug 31, 2022

I don't understand the 1 failing test

@Akuli
Copy link
Collaborator

Akuli commented Aug 31, 2022

stubtest is a tool that tries to import pyautogui in CI, to check whether your stubs match what is available with importing. It fails, because the CI environment doesn't have a screen attached to it (which is needed for using pyautogui; the way this works on Linux is that a screen name is stored in the $DISPLAY environment variable that it fails to find).

To silence the CI errors, you need to create a folder stubs/PyAutoGUI/@tests and add a file stubtest_allowlist.txt into it. There's an example of what to put into it at the end of the CI output. Many other stubs also have a @tests directory.

@github-actions

This comment has been minimized.

Avasam added a commit to Avasam/AutoSplit that referenced this pull request Aug 31, 2022
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.

Thank you! Here's a partial review -- I've got up to line 75 in __init__.pyi, will carry on later if somebody else doesn't beat me to it :)

@@ -90,7 +90,8 @@
"stubs/tzlocal/tzlocal/utils.pyi",
"stubs/ttkthemes",
"stubs/urllib3",
"stubs/vobject"
"stubs/vobject",
"stubs/PyAutoGUI"
Copy link
Member

Choose a reason for hiding this comment

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

Nit: can we keep this list alphabetised? :)

Copy link
Collaborator

@Akuli Akuli Aug 31, 2022

Choose a reason for hiding this comment

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

We could edit the stub generating script to keep it alphabetized. Alphabetizing would also help prevent merge conflicts: if two new items are added to the end, they will conflict, but if they are added to a random-ish place depending on the name of the project, they won't.

Copy link
Member

Choose a reason for hiding this comment

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

We could edit the stub generating script to keep it alphabetized.

I like that idea :)

Copy link
Member

@AlexWaygood AlexWaygood Sep 2, 2022

Choose a reason for hiding this comment

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

(Actually, it looks like the stub-generating script already has logic in it to keep the list alphabetised. I'm guessing the only reason it wasn't initially alphabetised here is because of the path-separator issue being discussed in #8653, which screwed up the alphabetisation logic. So I think this issue will be fixed when #8653 is merged.)

stubs/PyAutoGUI/pyautogui/__init__.pyi Outdated Show resolved Hide resolved
stubs/PyAutoGUI/pyautogui/__init__.pyi Outdated Show resolved Hide resolved
stubs/PyAutoGUI/pyautogui/__init__.pyi Show resolved Hide resolved
stubs/PyAutoGUI/pyautogui/__init__.pyi Outdated Show resolved Hide resolved
stubs/PyAutoGUI/pyautogui/__init__.pyi Outdated Show resolved Hide resolved
@github-actions

This comment has been minimized.

@Avasam
Copy link
Sponsor Collaborator Author

Avasam commented Aug 31, 2022

I ran stubtest locally so it could tell me about missing class members without the display error. However, just like with my D3DShot PR, I'm not sure how to deal with pointer types. ie dwExtraInfo: ctypes.POINTER(ctypes.wintypes.ULONG)

Edit: I found the pointer types in ctypes.wintypes

@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.

Here's a review for the rest of the PR!

As well as the remarks inline here: we usually only add stubs for a package's public API, unless there's a good reason to add stubs for implementation details. That means we usually omit stubs for private modules like _pyautogui_osx.pyi, _pyautogui_java.pyi, _pyautogui_win.pyi and _pyautogui_x11.pyi. Would you mind removing those? (Add an allowlist entry if stubtest complains!)

The remarks I made in relation to the mouseDown() signature also apply to mouseUp(), click(), leftClick(), rightClick(), middleClick(), doubleClick(), tripleClick(), scroll(), hscroll(), vscroll(), moveTo(), moveRel(), dragTo() and dragRel().

The remarks I made in relation to press() also apply to hold() and typewrite().

stubs/PyAutoGUI/pyautogui/__init__.pyi Outdated Show resolved Hide resolved
stubs/PyAutoGUI/pyautogui/__init__.pyi Outdated Show resolved Hide resolved
stubs/PyAutoGUI/pyautogui/__init__.pyi Outdated Show resolved Hide resolved
stubs/PyAutoGUI/pyautogui/__init__.pyi Show resolved Hide resolved
stubs/PyAutoGUI/pyautogui/__init__.pyi Outdated Show resolved Hide resolved
stubs/PyAutoGUI/pyautogui/__init__.pyi Outdated Show resolved Hide resolved
stubs/PyAutoGUI/pyautogui/__init__.pyi Outdated Show resolved Hide resolved
@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.

Thank you!

stubs/PyAutoGUI/pyautogui/__init__.pyi Outdated Show resolved Hide resolved
stubs/PyAutoGUI/pyautogui/__init__.pyi Show resolved Hide resolved
@github-actions
Copy link
Contributor

github-actions bot commented Sep 3, 2022

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

@AlexWaygood AlexWaygood merged commit 4c100fe into python:master Sep 3, 2022
@Avasam Avasam deleted the pyautogui branch September 3, 2022 14:17
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.

None yet

3 participants