Add an _Image protocol to tkinter. #4766
Conversation
I'm in the process of pulling a newer version of typeshed into Google and have come across some code that passes PIL.ImageTk.PhotoImage objects to tkinter.Label.configure(). This causes pytype to complain that configure expects tkinter.Image. Changing the method to instead accept a protocol fixes this. I created the protocol by checking what attributes and methods are shared between tkinter.Image and PIL.ImageTk.PhotoImage (https://github.com/python-pillow/Pillow/blob/0bb6a19cbf250867e708c3c09aa68e857621d272/src/PIL/ImageTk.py#L65).
srittau
left a comment
srittau
left a comment
There was a problem hiding this comment.
+1 for using a protocol, LGTM as is, but I'd appreciate it if you could add return types to the protocol.
|
|
||
| class _Image(Protocol): | ||
| tk: _tkinter.TkappType | ||
| def __del__(self) -> None: ... |
There was a problem hiding this comment.
__del__ being part of a protocol seems odd, does tk really require it?
There was a problem hiding this comment.
I...don't really know. If there's documentation anywhere on what an image class should define to be tkinter-compatible, I haven't been able to find it. I just included all the methods that are present on both tkinter.Image and PIL.ImageTk.PhotoImage (except __str__).
|
I think Every tkinter image has to have a meaningful |
|
I assume that it wouldn't do anything, since every class has a |
|
Sounds like I should remove |
I'm in the process of pulling a newer version of typeshed into Google
and have come across new type errors in code that passes
PIL.ImageTk.PhotoImage objects to tkinter.Label.configure().
Changing the method to instead accept a protocol fixes this.
I created the protocol by checking what attributes and methods are
shared between tkinter.Image and PIL.ImageTk.PhotoImage
(https://github.com/python-pillow/Pillow/blob/0bb6a19cbf250867e708c3c09aa68e857621d272/src/PIL/ImageTk.py#L65).