Skip to content

Conversation

@sobolevn
Copy link
Member

@github-actions

This comment has been minimized.

@sobolevn
Copy link
Member Author

СС @Akuli

@github-actions

This comment has been minimized.

@Akuli
Copy link
Collaborator

Akuli commented Nov 18, 2021

bool should also be accepted, because it works at runtime. Tkinter does add and '+' or '' which is an older way to spell '+' if add else ''.

@sobolevn
Copy link
Member Author

So, Literal['', '+'] | bool | None it is?

@Akuli
Copy link
Collaborator

Akuli commented Nov 18, 2021

Yes, I'm fine with that. The documentation still doesn't make any sense to me, but if you prefer to change typeshed rather than documentation, that's fine with me.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@Akuli
Copy link
Collaborator

Akuli commented Nov 18, 2021

There's still a lot missing. This is on your branch:

akuli@akuli-desktop:~/typeshed$ git grep 'add: bool' stdlib/tkinter/
stdlib/tkinter/__init__.pyi:        self, sequence: str | None = ..., func: Callable[[Event[Misc]], Any] | None = ..., add: bool | None = ...
stdlib/tkinter/__init__.pyi:    def bind_all(self, sequence: str | None, func: str, add: bool | None = ...) -> None: ...
stdlib/tkinter/__init__.pyi:    def bind_all(self, *, func: str, add: bool | None = ...) -> None: ...
stdlib/tkinter/__init__.pyi:        self, className: str, sequence: str | None = ..., func: Callable[[Event[Misc]], Any] | None = ..., add: bool | None = ...
stdlib/tkinter/__init__.pyi:    def bind_class(self, className: str, sequence: str | None, func: str, add: bool | None = ...) -> None: ...
stdlib/tkinter/__init__.pyi:    def bind_class(self, className: str, *, func: str, add: bool | None = ...) -> None: ...
stdlib/tkinter/__init__.pyi:        add: bool | None = ...,
stdlib/tkinter/__init__.pyi:    def tag_bind(self, tagOrId: str | int, sequence: str | None, func: str, add: bool | None = ...) -> None: ...
stdlib/tkinter/__init__.pyi:    def tag_bind(self, tagOrId: str | int, *, func: str, add: bool | None = ...) -> None: ...
stdlib/tkinter/__init__.pyi:        self, tagName: str, sequence: str | None, func: Callable[[Event[Text]], Any] | None, add: bool | None = ...
stdlib/tkinter/__init__.pyi:    def tag_bind(self, tagName: str, sequence: str | None, func: str, add: bool | None = ...) -> None: ...

@sobolevn
Copy link
Member Author

Is it the same for all bind_* methods and add parameter? Or are there any exceptions?
Looks like docs only cover '+' | '-' and bind: https://docs.python.org/3/library/tkinter.html#bindings-and-events

I am not really familiar with the internals.

@Akuli
Copy link
Collaborator

Akuli commented Nov 18, 2021

All bind_foo and foo_bind methods call _bind, which contains the add and '+' or '' code.

@sobolevn
Copy link
Member Author

sobolevn commented Nov 18, 2021

Or basically all methods that call ._bind() inside: https://github.com/python/cpython/blob/32959108f9c543e3cb9f2b68bbc782bddded6f42/Lib/tkinter/__init__.py#L1420

Thanks! I'm on it.

@Akuli
Copy link
Collaborator

Akuli commented Nov 18, 2021

tkinter.Canvas.tag_bind and tkinter.Text.tag_bind still use bool | None.

Copy link
Contributor

@JohnVillalovos JohnVillalovos left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for doing this so fast!

@github-actions

This comment has been minimized.

@sobolevn
Copy link
Member Author

Oh my, GitHub totally needs black integration 😒

@github-actions

This comment has been minimized.

2 similar comments
@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

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

@Akuli
Copy link
Collaborator

Akuli commented Nov 18, 2021

Oh my, GitHub totally needs black integration

Related: #5552

Copy link
Collaborator

@Akuli Akuli left a comment

Choose a reason for hiding this comment

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

I still think the docs are insane and should be changed instead, but this looks good to me.

@Akuli Akuli merged commit 3db6ac2 into python:master Nov 18, 2021
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.

tkinter: bind() method type-hint incorrectly has 'add' argument as a bool

4 participants