bpo-33289: Return RGB triplet of ints instead of floats from tkinter.colorchooser #6578

csabella · 2018-04-23T17:46:50Z

https://bugs.python.org/issue33289

serhiy-storchaka

Docstrings look excessive verbose.

Any chance of writing tests for the color chooser?

Lib/tkinter/__init__.py

Lib/tkinter/test/test_tkinter/test_misc.py

csabella · 2018-04-23T21:33:33Z

Any chance of writing tests for the color chooser?

The color chooser invokes the show() method which automatically displays the dialog. I couldn't figure out how to automate the test to click 'ok' or 'cancel' since I wasn't defining those myself. What's the best way to interact with that kind of dialog window?

Lib/tkinter/colorchooser.py

Lib/tkinter/test/test_tkinter/test_misc.py

bedevere-bot · 2018-04-25T22:38:25Z

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.

serhiy-storchaka · 2018-05-13T10:07:38Z

@csabella, do you mind to add a simple test as @terryjreedy required? I don't like tests that depend on implementation details, but here we don't have other way.

csabella · 2018-05-16T12:43:43Z

I have made the requested changes; please review again.

bedevere-bot · 2018-05-16T12:43:45Z

Thanks for making the requested changes!

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

Lib/tkinter/test/test_tkinter/test_colorchooser.py

terryjreedy

Running test_tk runs the two ChooserTests. They pass and look complete. Anything else is up to Serhiy.

Lib/tkinter/test/test_tkinter/test_misc.py

Lib/tkinter/test/test_tkinter/test_colorchooser.py

bedevere-bot · 2018-07-12T14:37:44Z

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.

terryjreedy · 2020-12-25T22:54:32Z

Cheryl has not been active for some months, so I am fixing the conflict. It is mostly in the imports. I made the minimal edits needed; you might want to review the import section.

terryjreedy · 2020-12-28T02:47:38Z

Pipelines Ubuntu:
======================================================================
FAIL: test_winfo_rgb (tkinter.test.test_tkinter.test_misc.MiscTest)
----------------------------------------------------------------------
Traceback (most recent call last):
18504
- (19018, 15420, 35980)
+ (18504, 15677, 35723)
======================================================================
FAIL: test_fixresult (tkinter.test.test_tkinter.test_colorchooser.ChooserTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vsts/work/1/s/Lib/tkinter/test/test_tkinter/test_colorchooser.py", line 38, in test_fixresult
    self.assertEqual(cc._fixresult(self.root, '#1e90ff'),
AssertionError: Tuples differ: ((33, 142, 255), '#1e90ff') != ((30, 144, 255), '#1e90ff')

First differing element 0:
(33, 142, 255)
(30, 144, 255)
- ((33, 142, 255), '#1e90ff')
?    ^    ^
+ ((30, 144, 255), '#1e90ff')
?    ^    ^

terryjreedy · 2021-01-21T09:21:51Z

On tracker msg385400 (today, on the bpo issue) I give preliminary experimental results and speculations about the test failures. I necessary, I will add some temporary debug prints to the tests.

Lib/tkinter/test/test_tkinter/test_misc.py

Lib/tkinter/test/test_tkinter/test_colorchooser.py

Lib/tkinter/test/test_tkinter/test_misc.py

Lib/tkinter/test/test_tkinter/test_colorchooser.py

Lib/tkinter/test/test_tkinter/test_misc.py

Lib/tkinter/__init__.py

Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>

Lib/tkinter/test/test_tkinter/test_misc.py

miss-islington · 2021-01-21T19:14:07Z

Thanks @csabella for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9.
🐍🍒⛏🤖

miss-islington · 2021-01-21T19:14:55Z

Sorry, @csabella and @serhiy-storchaka, I could not cleanly backport this to 3.9 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 6713e869c4989c04318158b406c30a147ea52904 3.9

miss-islington · 2021-01-21T19:14:57Z

Sorry @csabella and @serhiy-storchaka, I had trouble checking out the 3.8 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 6713e869c4989c04318158b406c30a147ea52904 3.8

bedevere-bot · 2021-01-24T21:45:52Z

GH-24318 is a backport of this pull request to the 3.9 branch.

…inter.colorchooser (pythonGH-6578). (cherry picked from commit 6713e86) Co-authored-by: Cheryl Sabella <cheryl.sabella@gmail.com>

…inter.colorchooser (pythonGH-6578). (pythonGH-24318) (cherry picked from commit 6713e86) Co-authored-by: Cheryl Sabella <cheryl.sabella@gmail.com> (cherry picked from commit 3d5434d) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

…inter.colorchooser (GH-6578). (GH-24318) (cherry picked from commit 6713e86) Co-authored-by: Cheryl Sabella <cheryl.sabella@gmail.com>

…inter.colorchooser (GH-6578). (GH-24318) (cherry picked from commit 6713e86) Co-authored-by: Cheryl Sabella <cheryl.sabella@gmail.com> (cherry picked from commit 3d5434d) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

…colorchooser (pythonGH-6578)

the-knights-who-say-ni added the CLA signed label Apr 23, 2018

bedevere-bot added the awaiting review label Apr 23, 2018

serhiy-storchaka reviewed Apr 23, 2018

View changes

Lib/tkinter/__init__.py Outdated Show resolved Hide resolved

Lib/tkinter/__init__.py Outdated Show resolved Hide resolved

Lib/tkinter/test/test_tkinter/test_misc.py Show resolved Hide resolved

Lib/tkinter/test/test_tkinter/test_misc.py Outdated Show resolved Hide resolved

serhiy-storchaka requested a review from terryjreedy Apr 25, 2018

terryjreedy requested changes Apr 25, 2018

View changes

Lib/tkinter/colorchooser.py Outdated Show resolved Hide resolved

Lib/tkinter/test/test_tkinter/test_misc.py Outdated Show resolved Hide resolved

bedevere-bot added awaiting changes and removed awaiting review labels Apr 25, 2018

bedevere-bot added awaiting change review and removed awaiting changes labels May 16, 2018

serhiy-storchaka reviewed May 16, 2018

View changes

Lib/tkinter/test/test_tkinter/test_colorchooser.py Outdated Show resolved Hide resolved

terryjreedy approved these changes May 16, 2018

View changes

bedevere-bot added awaiting merge and removed awaiting change review labels May 16, 2018

serhiy-storchaka requested changes Jul 12, 2018

View changes

bedevere-bot removed the awaiting merge label Jul 12, 2018

bedevere-bot added the awaiting changes label Jul 12, 2018

csabella added 3 commits Feb 9, 2020

bpo-33289: Return RGB triplet of ints from tkinter.colorchooser

324b39f

Clearer tests for winfo_rgb.

179d3a2

Add missing test_colorchooser source

d936fe5

csabella force-pushed the bpo33289 branch from 597495e to d936fe5 Compare Feb 9, 2020

csabella added 2 commits Feb 9, 2020

Use better examples in tests

e0c9cec

Remove gc_collect

a740841

This comment has been minimized.

Sign in to view

Use different colors in tests

9682699

Merge branch 'master' into bpo33289

d98349e

serhiy-storchaka approved these changes Dec 26, 2020

View changes

serhiy-storchaka reviewed Jan 21, 2021

View changes

Lib/tkinter/test/test_tkinter/test_misc.py Outdated Show resolved Hide resolved

Lib/tkinter/test/test_tkinter/test_colorchooser.py Outdated Show resolved Hide resolved

Apply suggestions from code review

d465142

serhiy-storchaka reviewed Jan 21, 2021

View changes

Lib/tkinter/test/test_tkinter/test_misc.py Outdated Show resolved Hide resolved

Lib/tkinter/test/test_tkinter/test_colorchooser.py Outdated Show resolved Hide resolved

Apply suggestions from code review

5b325b3

serhiy-storchaka reviewed Jan 21, 2021

View changes

Lib/tkinter/test/test_tkinter/test_misc.py Outdated Show resolved Hide resolved

Update Lib/tkinter/test/test_tkinter/test_misc.py

c092dbf

terryjreedy reviewed Jan 21, 2021

View changes

Lib/tkinter/__init__.py Outdated Show resolved Hide resolved

Update Lib/tkinter/__init__.py

8dcbfa1

Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>

serhiy-storchaka reviewed Jan 21, 2021

View changes

Lib/tkinter/test/test_tkinter/test_misc.py Outdated Show resolved Hide resolved

Update Lib/tkinter/test/test_tkinter/test_misc.py

b968e02

serhiy-storchaka merged commit 6713e86 into python:master Jan 21, 2021
3 checks passed

bedevere-bot removed the awaiting merge label Jan 21, 2021

miss-islington assigned serhiy-storchaka Jan 21, 2021

bedevere-bot removed the needs backport to 3.9 label Jan 24, 2021

adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021

bpo-33289: Return RGB triplet of ints instead of floats from tkinter.…

5ebd2c3

…colorchooser (pythonGH-6578)

TeamSpen210 mentioned this pull request Aug 21, 2022

Add types to tkinter.colorchooser.askcolor python/typeshed#8588

Merged

bpo-33289: Return RGB triplet of ints instead of floats from tkinter.colorchooser #6578

bpo-33289: Return RGB triplet of ints instead of floats from tkinter.colorchooser #6578

csabella commented Apr 23, 2018 •

edited by bedevere-bot

serhiy-storchaka left a comment

csabella commented Apr 23, 2018

bedevere-bot commented Apr 25, 2018

serhiy-storchaka commented May 13, 2018

csabella commented May 16, 2018

bedevere-bot commented May 16, 2018

terryjreedy left a comment

bedevere-bot commented Jul 12, 2018

This comment has been minimized.

terryjreedy commented Dec 25, 2020 •

edited

terryjreedy commented Dec 28, 2020 •

edited

terryjreedy commented Jan 21, 2021 •

edited

miss-islington commented Jan 21, 2021

miss-islington commented Jan 21, 2021

miss-islington commented Jan 21, 2021

bedevere-bot commented Jan 24, 2021

bpo-33289: Return RGB triplet of ints instead of floats from tkinter.colorchooser #6578

bpo-33289: Return RGB triplet of ints instead of floats from tkinter.colorchooser #6578

Conversation

csabella commented Apr 23, 2018 • edited by bedevere-bot

serhiy-storchaka left a comment

csabella commented Apr 23, 2018

bedevere-bot commented Apr 25, 2018

serhiy-storchaka commented May 13, 2018

csabella commented May 16, 2018

bedevere-bot commented May 16, 2018

terryjreedy left a comment

bedevere-bot commented Jul 12, 2018

This comment has been minimized.

terryjreedy commented Dec 25, 2020 • edited

terryjreedy commented Dec 28, 2020 • edited

terryjreedy commented Jan 21, 2021 • edited

miss-islington commented Jan 21, 2021

miss-islington commented Jan 21, 2021

miss-islington commented Jan 21, 2021

bedevere-bot commented Jan 24, 2021

csabella commented Apr 23, 2018 •

edited by bedevere-bot

terryjreedy commented Dec 25, 2020 •

edited

terryjreedy commented Dec 28, 2020 •

edited

terryjreedy commented Jan 21, 2021 •

edited