Skip to content

make tkinter.Event generic and add missing type hints to bind methods #4347

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
Jul 25, 2020

Conversation

Akuli
Copy link
Collaborator

@Akuli Akuli commented Jul 20, 2020

  • tkinter.Event is now generic, so tkinter.Event[tkinter.Entry] denotes an event coming from an Entry widget (e.g. user typed text to entry or clicked the entry).
  • Bind callbacks are now required to return Literal['break'] or None. At runtime, almost any return value is acceptable (more precisely, anything that could be converted to a Tcl object), and anything except the string 'break' would be treated the same. This breaks code that declares the return type as Optional[str]. If this is too bad, I can change Literal['break'] to str.
  • Plain tkinter.Event without specifying a widget type is supported for backwards compatibility, but unfortunately that's equivalent to tkinter.Event[tkinter.Any] rather than tkinter.Event[tkinter.Misc], so event.widget will in this case be Any-typed. It was previously typed Misc rather than any. This shouldn't cause backwards compatibility issues because nobody is using tkinter with mypy --strict (that just doesn't work with what's currently in typeshed).
  • bind() return value and other arguments are no longer Any typed.
  • Similar changes were done to all binding methods.

@Akuli
Copy link
Collaborator Author

Akuli commented Jul 20, 2020

there's conversation related to this in #4200

@Akuli
Copy link
Collaborator Author

Akuli commented Jul 20, 2020

Currently CI checks are failing because these overloads overlap:

    @overload
    def bind(
        self,
        sequence: Optional[str] = ...,
        func: Optional[Callable[[Event[Misc]], Optional[Literal["break"]]]] = ...,
        add: Optional[bool] = ...,
    ) -> str: ...
    @overload
    def bind(self, sequence: Optional[str] = ..., func: str = ..., add: Optional[bool] = ...) -> None: ...

I believe widget.bind('<Button-1>') is an example of an overlapping call. I want that to return str. How should I do this?

@srittau
Copy link
Collaborator

srittau commented Jul 21, 2020

As far as I understand, you would want to use the following overload (i.e. no default for func):

    @overload
    def bind(self, sequence: Optional[str] = ..., func: str, add: Optional[bool] = ...) -> None: ...

Since this isn't possible, you need to use two overloads:

    @overload
    def bind(self, sequence: Optional[str], func: str, add: Optional[bool] = ...) -> None: ...
    @overload
    def bind(self, *, func: str, add: Optional[bool] = ...) -> None: ...

@Akuli
Copy link
Collaborator Author

Akuli commented Jul 22, 2020

@Hawk777 What's your opinion on Optional[Literal['break']] vs Optinal[str] and backwards compatibility?

@Hawk777
Copy link
Contributor

Hawk777 commented Jul 22, 2020

Personally, I like Optional[Literal['break']]. It’s not like linters in general promise to never add new warnings! Sure, it will make existing code fail mypy, but it won’t break actual execution of existing code because execution ignores the type hints anyway.

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.

I have no experience with tk, but the changes look sensible to me.

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.

3 participants