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

Depend on Pillow instead of types-Pillow #11720

Merged
merged 5 commits into from
Apr 5, 2024
Merged

Conversation

srittau
Copy link
Collaborator

@srittau srittau commented Apr 5, 2024

No description provided.

@srittau
Copy link
Collaborator Author

srittau commented Apr 5, 2024

The PIL imports are trivial in all those packages: from PIL import Image. This doesn't prevent users from installing types-Pillow in addition to Pillow.

The PyAutoGUI stubs don't import PIL directly. But they depend on types-PyScreeze, which itself depends on Pillow, so any indirect "Pillow" needs in types-PyAutoGUI should be handled by indirect dependency.

@srittau
Copy link
Collaborator Author

srittau commented Apr 5, 2024

Cf. #11688.

@AlexWaygood Considering we should start depending on Pillow instead of types-Pillow, working around the problems in the stubs instead of our test infrastructure seems to be the easiest solution.

@AlexWaygood

This comment was marked as resolved.

This comment has been minimized.

@AlexWaygood
Copy link
Member

Looks great, thanks!

@srittau
Copy link
Collaborator Author

srittau commented Apr 5, 2024

Some annotations in the Pillow annotations are still lacking, compared to typeshed. But they are enough for the stubs that depend on it, since they only use the Image class, which obviously exist in Pillow as well. By using the dependency on Pillow instead of types-Pillow, our users can now decide whether Pillow is enough or they still want to use types-Pillow.

@AlexWaygood
Copy link
Member

Note that this does mean that we will no longer have any pyright coverage for our Pillow stubs in CI (it will only see the inline Pillow types, not our stubs). Still, if we now plan on contributing improvements upstream to Pillow and marking our stubs obsolete soon, that shouldn't be a major issue. (I endorse the plan, though I'm afraid I can't commit to helping out with it much over the next few weeks ☹️)

This comment has been minimized.

@AlexWaygood AlexWaygood closed this Apr 5, 2024
@AlexWaygood AlexWaygood reopened this Apr 5, 2024
@srittau
Copy link
Collaborator Author

srittau commented Apr 5, 2024

This stub uploader check seems to be overzealous. In this case, python-xlib doesn't need to depend on Pillow to be able to work with Pillow's Image objects. Our stubs would need to depend on it though.

I'll try a different solution, although I'm not sure that works in stubs.

@AlexWaygood
Copy link
Member

It looks like our python-xlib stubs might only use the types-Pillow dependency for a single annotation? It probably wouldn't be the worst thing in the world if that just changed to Any

This comment has been minimized.

@srittau
Copy link
Collaborator Author

srittau commented Apr 5, 2024

Yeah, that's the best solution for now. I'll open a discussion about conditional imports in stub files, though.

Copy link
Contributor

github-actions bot commented Apr 5, 2024

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

@srittau srittau merged commit c0a44be into python:main Apr 5, 2024
43 checks passed
@srittau srittau deleted the pillow branch April 5, 2024 11:30
This pull request was closed.
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