bpo-33289: Return RGB triplet of ints instead of floats from tkinter.colorchooser by csabella · Pull Request #6578 · python/cpython

csabella · Apr 23, 2018

https://bugs.python.org/issue33289

serhiy-storchaka

Docstrings look excessive verbose.

Any chance of writing tests for the color chooser?

csabella · Apr 23, 2018

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?

bedevere-bot · Apr 25, 2018

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 · May 13, 2018

@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 · May 16, 2018

I have made the requested changes; please review again.

bedevere-bot · May 16, 2018

Thanks for making the requested changes!

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

terryjreedy

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

bedevere-bot · Jul 12, 2018

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 · Dec 28, 2020

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 · Jan 21, 2021

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.

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

miss-islington · Jan 21, 2021

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 · Jan 21, 2021

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 · Jan 21, 2021

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 · Jan 24, 2021

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 reviewed changes

Comment thread Lib/tkinter/__init__.py Outdated

Comment thread Lib/tkinter/__init__.py Outdated

Comment thread Lib/tkinter/test/test_tkinter/test_misc.py

Comment thread Lib/tkinter/test/test_tkinter/test_misc.py Outdated

serhiy-storchaka requested a review from terryjreedy April 25, 2018 17:56

terryjreedy requested changes Apr 25, 2018

View reviewed changes

Comment thread Lib/tkinter/colorchooser.py Outdated

Comment thread Lib/tkinter/test/test_tkinter/test_misc.py

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 reviewed changes

Comment thread Lib/tkinter/test/test_tkinter/test_colorchooser.py Outdated

terryjreedy approved these changes May 16, 2018

View reviewed changes

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

serhiy-storchaka requested changes Jul 12, 2018

View reviewed 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 February 9, 2020 14:51

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 February 9, 2020 19:56

csabella added 2 commits February 9, 2020 15:27

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

serhiy-storchaka approved these changes Dec 26, 2020

View reviewed changes

serhiy-storchaka reviewed Jan 21, 2021

View reviewed changes

Comment thread Lib/tkinter/test/test_tkinter/test_misc.py Outdated

Comment thread Lib/tkinter/test/test_tkinter/test_colorchooser.py Outdated

Apply suggestions from code review

d465142

serhiy-storchaka reviewed Jan 21, 2021

View reviewed changes

Comment thread Lib/tkinter/test/test_tkinter/test_misc.py Outdated

Comment thread Lib/tkinter/test/test_tkinter/test_colorchooser.py Outdated

Apply suggestions from code review

5b325b3

serhiy-storchaka reviewed Jan 21, 2021

View reviewed changes

Comment thread Lib/tkinter/test/test_tkinter/test_misc.py Outdated

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

c092dbf

terryjreedy reviewed Jan 21, 2021

View reviewed changes

Comment thread Lib/tkinter/__init__.py Outdated

Update Lib/tkinter/__init__.py

8dcbfa1

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

serhiy-storchaka reviewed Jan 21, 2021

View reviewed changes

Comment thread Lib/tkinter/test/test_tkinter/test_misc.py Outdated

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

b968e02

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

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

serhiy-storchaka removed their assignment Sep 29, 2024

serhiy-storchaka removed the needs backport to 3.8 label Sep 29, 2024

Search code, repositories, users, issues, pull requests...

Uh oh!

Uh oh!

Conversation

csabella commented Apr 23, 2018 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

csabella commented Apr 23, 2018

Uh oh!

Uh oh!

Uh oh!

bedevere-bot commented Apr 25, 2018

Uh oh!

serhiy-storchaka commented May 13, 2018

Uh oh!

csabella commented May 16, 2018

Uh oh!

bedevere-bot commented May 16, 2018

Uh oh!

Uh oh!

terryjreedy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bedevere-bot commented Jul 12, 2018

Uh oh!

This comment has been minimized.

terryjreedy commented Dec 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

terryjreedy commented Jan 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

miss-islington commented Jan 21, 2021

Uh oh!

miss-islington commented Jan 21, 2021

Uh oh!

miss-islington commented Jan 21, 2021

Uh oh!

bedevere-bot commented Jan 24, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

csabella commented Apr 23, 2018 •

edited by bedevere-bot

Loading

terryjreedy commented Dec 28, 2020 •

edited

Loading

terryjreedy commented Jan 21, 2021 •

edited

Loading