Skip to content

gh-126899: Add **kw to tkinter.Misc.after and tkinter.Misc.after_idle #126900

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 11 commits into from
Dec 1, 2024

Conversation

Xiaokang2022
Copy link
Contributor

@Xiaokang2022 Xiaokang2022 commented Nov 16, 2024

@graingert
Copy link
Contributor

For this to work you need to make func and ms positional only, which is backwards incompatible

@Xiaokang2022
Copy link
Contributor Author

For this to work you need to make func and ms positional only, which is backwards incompatible

Is making func and ms positional only a must? If not, it will not lead to incompatibilities.

I've found something similar elsewhere in tkinter, but it doesn't enforce that the preceding argument to positional only:

def element_create(self, elementname, etype, *args, **kw):

and

def instate(self, statespec, callback=None, *args, **kw):

@serhiy-storchaka
Copy link
Member

It is not necessary to make other parameters positional only, but this can be considered in future, together with other methods that have var-keyword parameter.

In general, I do not see objections against this change. It does not also look that other Tkinter methods need similar change. But please add tests and a What's New entry.

@Xiaokang2022
Copy link
Contributor Author

@serhiy-storchaka PTAL, thanks!

@Xiaokang2022
Copy link
Contributor Author

In any case this needs a review by a native English speaker.

@terryjreedy , As mentioned above, I guess you may be able to help us review this PR, it's easy, thanks!

Copy link
Member

@terryjreedy terryjreedy left a comment

Choose a reason for hiding this comment

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

I leave code changes to Serhiy.

@bedevere-app
Copy link

bedevere-app bot commented Dec 1, 2024

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

The code LGTM.

@Xiaokang2022
Copy link
Contributor Author

I have made the requested changes; please review again.

@bedevere-app
Copy link

bedevere-app bot commented Dec 1, 2024

Thanks for making the requested changes!

@terryjreedy: please review the changes made to this pull request.

@bedevere-app bedevere-app bot requested a review from terryjreedy December 1, 2024 12:00
@Xiaokang2022
Copy link
Contributor Author

I don't know why the test failed. Compared to the last commit, I only modified the parts that belonged to the documentation.

@terryjreedy
Copy link
Member

terryjreedy commented Dec 1, 2024

There was a (rare) timeout error in building Python for the test. Nothing to do with us. Re-running the failed test only.
EDIT: Build phase already finished in rerun.

@terryjreedy terryjreedy enabled auto-merge (squash) December 1, 2024 19:17
@terryjreedy terryjreedy merged commit 7ea523f into python:main Dec 1, 2024
43 of 46 checks passed
picnixz pushed a commit to picnixz/cpython that referenced this pull request Dec 2, 2024
….after_idle` (python#126900)


---------
Co-authored-by: Serhiy Storchaka <[email protected]>
@Xiaokang2022 Xiaokang2022 deleted the improve-tkinter-after branch January 6, 2025 04:39
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Jan 8, 2025
….after_idle` (python#126900)


---------
Co-authored-by: Serhiy Storchaka <[email protected]>
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
….after_idle` (python#126900)


---------
Co-authored-by: Serhiy Storchaka <[email protected]>
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.

5 participants