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

More complete type stubs for tkinter Canvas methods. #5188

Merged
merged 15 commits into from
Apr 22, 2021

Conversation

DanielRosenwasser
Copy link
Contributor

This change

  • Establishes a NewType called _ObjectId, meant to ensure that arbitrary handles aren't created.
  • Defines more specific method declarations for Canvas instances.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 8, 2021

Diff from mypy_primer, showing the effect of this PR on open source code:

porcupine (https://github.com/Akuli/porcupine.git)
+ /tmp/mypy_primer/new_typeshed/typeshed_to_test/stdlib/tkinter/__init__.pyi:1170: error: invalid syntax  [syntax]
- porcupine/utils.py:22: error: Cannot find implementation or library stub for module named "dacite"  [import]
- porcupine/settings.py:17: error: Cannot find implementation or library stub for module named "dacite"  [import]
- porcupine/settings.py:80: error: unused 'type: ignore' comment
- porcupine/plugins/langserver.py:29: error: Cannot find implementation or library stub for module named "sansio_lsp_client"  [import]
- porcupine/plugins/filetypes.py:11: error: Library stubs not installed for "toml" (or incompatible with Python 3.8)  [import]
- porcupine/plugins/filetypes.py:11: note: (or run "mypy --install-types" to install all missing stub packages)
- porcupine/plugins/filetypes.py:11: note: Hint: "python3 -m pip install types-toml"
- porcupine/plugins/filetypes.py:11: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports

@hauntsaninja
Copy link
Collaborator

It'll be a couple years before typeshed can use PEP 570 syntax, for now positional-only arguments are indicated by prefixing with a double underscore.

@srittau
Copy link
Collaborator

srittau commented Apr 8, 2021

Except for some auto generate code for protobuf, this is the first time we use NewType in typeshed. Just something we need to watch out for after the change is published.

@hauntsaninja
Copy link
Collaborator

Wow, I cannot wait to be done with Python 3.5.1 :-)

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.

Thanks, remarks below! Also, running black on this file should hopefully fix the build issues.

stdlib/tkinter/__init__.pyi Outdated Show resolved Hide resolved
stdlib/tkinter/__init__.pyi Outdated Show resolved Hide resolved
stdlib/tkinter/__init__.pyi Outdated Show resolved Hide resolved
stdlib/tkinter/__init__.pyi Outdated Show resolved Hide resolved
stdlib/tkinter/__init__.pyi Outdated Show resolved Hide resolved
stdlib/tkinter/__init__.pyi Outdated Show resolved Hide resolved
stdlib/tkinter/__init__.pyi Outdated Show resolved Hide resolved
stdlib/tkinter/__init__.pyi Outdated Show resolved Hide resolved
stdlib/tkinter/__init__.pyi Outdated Show resolved Hide resolved
@DanielRosenwasser
Copy link
Contributor Author

I can't seem to get black installed on Windows for whatever reason, so I will hold off on that step if that's acceptable to you all.

@Akuli
Copy link
Collaborator

Akuli commented Apr 10, 2021

I blacked the code (pull before making more changes), but it is also possible on windows. I think something like:

py -m venv env
env\Scripts\activate.bat
pip install black
black stdlib

@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

porcupine (https://github.com/Akuli/porcupine.git)
+ porcupine/plugins/welcome.py:47: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/welcome.py:51: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/minimap.py:93: error: Argument "font" has incompatible type "Tuple[str, int, Tuple[]]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/linenumbers.py:47: error: Argument 1 to "delete" of "Canvas" has incompatible type "str"; expected "Union[_CanvasObjectId, Iterable[_CanvasObjectId]]"  [arg-type]
+ more_plugins/tetris.py:307: error: Argument 1 to "delete" of "Canvas" has incompatible type "int"; expected "Union[_CanvasObjectId, Iterable[_CanvasObjectId]]"  [arg-type]
+ more_plugins/tetris.py:331: error: Argument "font" to "create_text" of "Canvas" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/fold.py:60: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/pluginmanager.py:64: error: Argument "columns" to "Treeview" has incompatible type "Tuple[str, str, str]"; expected "Union[str, Union[List[str], Tuple[str, Any]]]"  [arg-type]
+ porcupine/plugins/pluginmanager.py:93: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/pastebin.py:200: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, Tuple[]]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]

stdlib/tkinter/__init__.pyi Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

porcupine (https://github.com/Akuli/porcupine.git)
+ porcupine/plugins/welcome.py:47: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/welcome.py:51: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/minimap.py:93: error: Argument "font" has incompatible type "Tuple[str, int, Tuple[]]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/linenumbers.py:47: error: Argument 1 to "delete" of "Canvas" has incompatible type "str"; expected "Union[_CanvasObjectId, Iterable[_CanvasObjectId]]"  [arg-type]
+ more_plugins/tetris.py:307: error: Argument 1 to "delete" of "Canvas" has incompatible type "int"; expected "Union[_CanvasObjectId, Iterable[_CanvasObjectId]]"  [arg-type]
+ more_plugins/tetris.py:331: error: Argument "font" to "create_text" of "Canvas" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/fold.py:60: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/pluginmanager.py:64: error: Argument "columns" to "Treeview" has incompatible type "Tuple[str, str, str]"; expected "Union[str, Union[List[str], Tuple[str, Any]]]"  [arg-type]
+ porcupine/plugins/pluginmanager.py:93: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/pastebin.py:200: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, Tuple[]]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]

1 similar comment
@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

porcupine (https://github.com/Akuli/porcupine.git)
+ porcupine/plugins/welcome.py:47: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/welcome.py:51: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/minimap.py:93: error: Argument "font" has incompatible type "Tuple[str, int, Tuple[]]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/linenumbers.py:47: error: Argument 1 to "delete" of "Canvas" has incompatible type "str"; expected "Union[_CanvasObjectId, Iterable[_CanvasObjectId]]"  [arg-type]
+ more_plugins/tetris.py:307: error: Argument 1 to "delete" of "Canvas" has incompatible type "int"; expected "Union[_CanvasObjectId, Iterable[_CanvasObjectId]]"  [arg-type]
+ more_plugins/tetris.py:331: error: Argument "font" to "create_text" of "Canvas" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/fold.py:60: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/pluginmanager.py:64: error: Argument "columns" to "Treeview" has incompatible type "Tuple[str, str, str]"; expected "Union[str, Union[List[str], Tuple[str, Any]]]"  [arg-type]
+ porcupine/plugins/pluginmanager.py:93: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/pastebin.py:200: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, Tuple[]]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]

stdlib/tkinter/__init__.pyi Outdated Show resolved Hide resolved
@Akuli
Copy link
Collaborator

Akuli commented Apr 10, 2021

Can you find/replace tuple back to Tuple? The lower-case tuple seems to be causing mypy problems (#4820 ). Once those are no longer a problem, we can make a separate pull request that changes Tuple to tuple.

@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

porcupine (https://github.com/Akuli/porcupine.git)
+ porcupine/plugins/welcome.py:47: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/welcome.py:51: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/minimap.py:93: error: Argument "font" has incompatible type "Tuple[str, int, Tuple[]]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/linenumbers.py:47: error: Argument 1 to "delete" of "Canvas" has incompatible type "str"; expected "Union[_CanvasObjectId, Iterable[_CanvasObjectId]]"  [arg-type]
+ more_plugins/tetris.py:307: error: Argument 1 to "delete" of "Canvas" has incompatible type "int"; expected "Union[_CanvasObjectId, Iterable[_CanvasObjectId]]"  [arg-type]
+ more_plugins/tetris.py:331: error: Argument "font" to "create_text" of "Canvas" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/fold.py:60: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/pluginmanager.py:64: error: Argument "columns" to "Treeview" has incompatible type "Tuple[str, str, str]"; expected "Union[str, Union[List[str], Tuple[str, Any]]]"  [arg-type]
+ porcupine/plugins/pluginmanager.py:93: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/pastebin.py:200: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, Tuple[]]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]

1 similar comment
@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

porcupine (https://github.com/Akuli/porcupine.git)
+ porcupine/plugins/welcome.py:47: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/welcome.py:51: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/minimap.py:93: error: Argument "font" has incompatible type "Tuple[str, int, Tuple[]]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/linenumbers.py:47: error: Argument 1 to "delete" of "Canvas" has incompatible type "str"; expected "Union[_CanvasObjectId, Iterable[_CanvasObjectId]]"  [arg-type]
+ more_plugins/tetris.py:307: error: Argument 1 to "delete" of "Canvas" has incompatible type "int"; expected "Union[_CanvasObjectId, Iterable[_CanvasObjectId]]"  [arg-type]
+ more_plugins/tetris.py:331: error: Argument "font" to "create_text" of "Canvas" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/fold.py:60: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/pluginmanager.py:64: error: Argument "columns" to "Treeview" has incompatible type "Tuple[str, str, str]"; expected "Union[str, Union[List[str], Tuple[str, Any]]]"  [arg-type]
+ porcupine/plugins/pluginmanager.py:93: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, str]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]
+ porcupine/plugins/pastebin.py:200: error: Argument "font" to "Label" has incompatible type "Tuple[str, int, Tuple[]]"; expected "Union[str, Font, Union[List[Any], Tuple[Any, Any]]]"  [arg-type]

@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

porcupine (https://github.com/Akuli/porcupine.git)
+ porcupine/plugins/linenumbers.py:47: error: Argument 1 to "delete" of "Canvas" has incompatible type "str"; expected "Union[_CanvasObjectId, Iterable[_CanvasObjectId]]"  [arg-type]
+ more_plugins/tetris.py:307: error: Argument 1 to "delete" of "Canvas" has incompatible type "int"; expected "Union[_CanvasObjectId, Iterable[_CanvasObjectId]]"  [arg-type]

@Akuli
Copy link
Collaborator

Akuli commented Apr 11, 2021

Often you can use a tag string instead of an item ID. The remaining mypy_primer failures are from code that does canvas.delete('all'), where 'all' is a tag string. That should be accepted whenever the manual page says tagOrId.

To fix this, you can replace _CanvasObjectId with Union[str, _CanvasObjectId] in a few places, or add a type alias:

_TagOrId = Union[_CanvasObjectId, str]

@Akuli
Copy link
Collaborator

Akuli commented Apr 17, 2021

@DanielRosenwasser Are you still interested in working on this? There's no hurry, but I can do the rest if you want.

@DanielRosenwasser
Copy link
Contributor Author

Hey @Akuli let me know if there's anything I've missed in the latest changes.

@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

porcupine (https://github.com/Akuli/porcupine.git)
+ more_plugins/tetris.py:307: error: Argument 1 to "delete" of "Canvas" has incompatible type "int"; expected "Union[str, _CanvasItemId]"  [arg-type]

@Akuli
Copy link
Collaborator

Akuli commented Apr 18, 2021

The mypy_primer failure comes from code that (I wrote and) uses int as the type for canvas items. What would be the recommended way once this PR is merged? We have a few options:

  • Try to have mypy automatically determine the type (not possible in this case)
  • Use tkinter._CanvasItemId (I don't think this works, and it's fragile and not discoverable, not guessable, not documented anywhere)
  • Rename it to CanvasItemId and use that (would work, but still not discoverable/guessable/documented)
  • Just get rid of NewType and everything will work

What do you think?

@DanielRosenwasser
Copy link
Contributor Author

I've been thinking about this a bit and it really depends on how much someone wants to protect against breaking the abstraction in addition to making sure you can't forge a fake object ID (on purpose or accidentally). In either case, it's not that important and unlikely that an ID will ever stop being an int.

@DanielRosenwasser
Copy link
Contributor Author

So I'll try to just turn it into a plain type alias to int later today.

@github-actions
Copy link
Contributor

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

1 similar comment
@github-actions
Copy link
Contributor

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

stdlib/tkinter/__init__.pyi Outdated Show resolved Hide resolved
@Akuli
Copy link
Collaborator

Akuli commented Apr 21, 2021

The code needs blacking again. Let me know whether you want to black it yourself or you want me to black it again.

DanielRosenwasser and others added 2 commits April 21, 2021 14:17
Co-authored-by: Akuli <akuviljanen17@gmail.com>
@Akuli
Copy link
Collaborator

Akuli commented Apr 21, 2021

I decided to black it

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.

Thanks!

@DanielRosenwasser
Copy link
Contributor Author

Ah just as I was figuring out how to venv 😄

Thanks!

@github-actions
Copy link
Contributor

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

1 similar comment
@github-actions
Copy link
Contributor

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

@Akuli Akuli merged commit 863b8f2 into python:master Apr 22, 2021
@Akuli Akuli mentioned this pull request Jun 6, 2021
@DanielRosenwasser DanielRosenwasser deleted the canvas branch April 19, 2024 18:35
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.

4 participants