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

bpo-41730: Show deprecation warnings for tkinter.tix #22186

Merged
merged 18 commits into from
May 7, 2021
Merged

Conversation

wyz23x2
Copy link
Contributor

@wyz23x2 wyz23x2 commented Sep 10, 2020

The tkinter.tix module has been deprecated since Python 3.6. Warn a DeprecationWarning when importing it.

https://bugs.python.org/issue41730

@wyz23x2 wyz23x2 changed the title bpo-41370: Show deprecation warnings for tkinter.tix bpo-41730: Show deprecation warnings for tkinter.tix Sep 10, 2020
@wyz23x2
Copy link
Contributor Author

wyz23x2 commented Sep 10, 2020

All tests have passed. Now it's time to merge!

Copy link
Contributor

@E-Paine E-Paine left a comment

Choose a reason for hiding this comment

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

  1. Good spot with the example (lines 16 + 17) 👍

  2. Your PR still shows in issue 41370

  3. The doc (https://docs.python.org/3/library/tkinter.tix.html) states "This Tk extension is unmaintained and should not be used in new code.".

    Apologies, I missed that.

Suggested by @E-Paine

Co-authored-by: E-Paine <63801254+E-Paine@users.noreply.github.com>
@wyz23x2
Copy link
Contributor Author

wyz23x2 commented Sep 10, 2020

Has been removed from bpo-41370. Thanks!

@E-Paine
Copy link
Contributor

E-Paine commented Sep 10, 2020

Has been removed from bpo 41370. Thanks!

Um... Not sure if this is because of bedevere on your last comment, but it has reappeared there (may just be worth leaving it for someone else to sort out...)

@wyz23x2
Copy link
Contributor Author

wyz23x2 commented Sep 10, 2020

😓

@wyz23x2
Copy link
Contributor Author

wyz23x2 commented Sep 10, 2020

@E-Paine
Copy link
Contributor

E-Paine commented Sep 10, 2020

It appears so 😄 (EDIT: Appears like its fixed rather than its still there - sorry for the lack of clarity)

@wyz23x2

This comment has been minimized.

@wyz23x2
Copy link
Contributor Author

wyz23x2 commented Sep 10, 2020

🆗! Thanks!

@wyz23x2
Copy link
Contributor Author

wyz23x2 commented Sep 10, 2020

IMO the tracker shouldn't link again if it was unlinked before 😄

@wyz23x2
Copy link
Contributor Author

wyz23x2 commented Mar 16, 2021

Um, is there any further process on this?

@bedevere-bot
Copy link

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.

wyz23x2 and others added 2 commits March 17, 2021 19:41
Co-authored-by: Zachary Ware <zachary.ware@gmail.com>
Co-authored-by: Zachary Ware <zachary.ware@gmail.com>
@bedevere-bot
Copy link

Thanks for making the requested changes!

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

@bedevere-bot bedevere-bot requested a review from zware May 7, 2021 15:12
@bedevere-bot
Copy link

Thanks for making the requested changes!

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

@E-Paine
Copy link
Contributor

E-Paine commented May 7, 2021

You also need to change line 32 to use import_helper

@E-Paine
Copy link
Contributor

E-Paine commented May 7, 2021

I believe we should keep the import _tkinter (in tix.py) as it reduces the length of the traceback (if tkinter fails to load for whatever reason).

@E-Paine
Copy link
Contributor

E-Paine commented May 7, 2021

Actually, it was redundant where it was. So we should probably move it before the main tkinter import or remove it (as you have done).

@zware zware added the needs backport to 3.10 only security fixes label May 7, 2021
@zware zware merged commit 4a2d98a into python:main May 7, 2021
@miss-islington
Copy link
Contributor

Thanks @wyz23x2 for the PR, and @zware for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-25971 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label May 7, 2021
@wyz23x2 wyz23x2 deleted the patch-5 branch May 7, 2021 16:06
miss-islington added a commit that referenced this pull request May 7, 2021
Co-authored-by: E-Paine <63801254+E-Paine@users.noreply.github.com>
Co-authored-by: Zachary Ware <zach@python.org>
(cherry picked from commit 4a2d98a)

Co-authored-by: wyz23x2 <52805709+wyz23x2@users.noreply.github.com>
@pablogsal
Copy link
Member

Unfortunately this commit has introduced some reference leaks, see:

https://buildbot.python.org/all/#/builders/684/builds/5

@zware could you take a look?

zware added a commit to zware/cpython that referenced this pull request May 9, 2021
Added in bpo-41730 (pythonGH-22186), the test apparently causes refleaks.  The
test isn't worth hunting them down, so it's simply reverted.

This partially reverts commit 4a2d98a.
@zware
Copy link
Member

zware commented May 9, 2021

The linked failure doesn't appear to have anything to do with Tix, and in fact the next build (not yet finished, on its last test) appears to be fine.

Ah, you mean https://buildbot.python.org/all/#/builders/511/builds/23 :). The test isn't worthwhile; I've opened GH-26005 to revert it.

zware added a commit that referenced this pull request May 9, 2021
Added in bpo-41730 (GH-22186), the test apparently causes refleaks.  The
test isn't worth hunting them down, so it's simply reverted.

This partially reverts commit 4a2d98a.
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 9, 2021
Added in bpo-41730 (pythonGH-22186), the test apparently causes refleaks.  The
test isn't worth hunting them down, so it's simply reverted.

This partially reverts commit 4a2d98a.
(cherry picked from commit 8e8307d)

Co-authored-by: Zachary Ware <zach@python.org>
miss-islington added a commit that referenced this pull request May 9, 2021
Added in bpo-41730 (GH-22186), the test apparently causes refleaks.  The
test isn't worth hunting them down, so it's simply reverted.

This partially reverts commit 4a2d98a.
(cherry picked from commit 8e8307d)

Co-authored-by: Zachary Ware <zach@python.org>
@pablogsal
Copy link
Member

Ah, you mean https://buildbot.python.org/all/#/builders/511/builds/23 :). The test isn't worthwhile; I've opened GH-26005 to revert it.

My bad! I copied the wrong url :(

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.

7 participants