Skip to content

Add defaults for tkinter __init__ methods #11391

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 15 commits into from
Feb 10, 2024

Conversation

MarcellPerger1
Copy link
Contributor

Add defaults for tkinter's __init__ methods, see #10947

The defaults I filled in are consistent across:

  • My local Windows 11 machine
  • All available github actions runners:
    • ubuntu-20.04, ubuntu-22.04
    • windows-2019, windows-2022
    • macos-11, macos-12, macos-13

I used a script to generate the defaults into a JSON file (source code at https://github.com/MarcellPerger1/typeshed-tkinter-defaults), then I filled them into the arguments by hand (I also checked them with the tcl.tk documentation to ensure the defaults make sense).

I tried to fill the defaults in a way that keeps the defaults value consistent with the argument type (while still keeping it close to the types returned by .cget).

Some notes about specific arguments:

  • variable and textvariable: The default value (using dict) is consistently "" but this doesn't match the annotated Variable type so I didn't fill these in
  • Message(padx, pady): The default value is -1 which is not allowed according to the documentation so I didn't fill this in
  • Toplevel(use): The default value is always "" but this doesn't match the annotated int type so I didn't fill this in

P.S. Sorry for the huge diff.

MarcellPerger1 and others added 8 commits February 3, 2024 22:34
NOTE: Listbox `exportselection` param type might be wrong, 
should perhaps be `bool`, not `int`, like all the other `exportselection` params
NOTE: not sure what to put for `Message(padx, pady)`.
The default is always `"-1"` but the pad value must be positive, see
https://tcl.tk/man/tcl8.6/TkCmd/options.htm#M-padx#:~:text=Command%2DLine%20Name%3A%20%2Dpadx,Specifies%20a%20non%2Dnegative%20value
Make it an actual `int` it is compatible with the `_TakeFocusValue` type
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 for doing this! For now I went through Toplevel and Button options manually.

@Akuli
Copy link
Collaborator

Akuli commented Feb 10, 2024

Are you going to make a separate PR that does this for tkinter.ttk widgets?

I mostly use the ttk widgets in my projects, because I can apply an awesome theme to them. I think a separate PR for ttk widgets would be easier to review than adding their defaults to this PR. This way, we won't get confused about tkinter.Button vs tkinter.ttk.Button, for example.

P.S. Sorry for the huge diff.

This is small compared to my biggest autogenerated tkinter PR :)

This comment has been minimized.

This comment has been minimized.

… a string

For the rest of the integer-in-strings, the default value is consistently a string on all platforms,
so I'm not sure if I should replace those with a plain integer (this would probably be more useful in IDEs but less accurate)
@MarcellPerger1
Copy link
Contributor Author

Are you going to make a separate PR that does this for tkinter.ttk widgets?

Yes

This comment has been minimized.

This comment has been minimized.

@MarcellPerger1 MarcellPerger1 marked this pull request as ready for review February 10, 2024 11:28
Copy link
Contributor

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

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! In separate PRs, we still need to add:

  • the same defaults for configure() methods
  • defaults for ttk widgets.

@Akuli Akuli merged commit 187928d into python:main Feb 10, 2024
@MarcellPerger1 MarcellPerger1 deleted the add-tkinter-defaults branch February 11, 2024 11: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.

3 participants