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

tkinter askcolor returning floats for r, g, b values instead of ints #77470

Closed
BryanOakley mannequin opened this issue Apr 16, 2018 · 14 comments
Closed

tkinter askcolor returning floats for r, g, b values instead of ints #77470

BryanOakley mannequin opened this issue Apr 16, 2018 · 14 comments
Labels
3.10 only security fixes topic-tkinter type-bug An unexpected behavior, bug, or error

Comments

@BryanOakley
Copy link
Mannequin

BryanOakley mannequin commented Apr 16, 2018

BPO 33289
Nosy @terryjreedy, @vstinner, @serhiy-storchaka, @csabella, @miss-islington
PRs
  • bpo-33289: Return RGB triplet of ints instead of floats from tkinter.colorchooser #6578
  • [3.9] bpo-33289: Return RGB triplet of ints instead of floats from tkinter.colorchooser (GH-6578). #24318
  • [3.8] [3.9] bpo-33289: Return RGB triplet of ints instead of floats from tkinter.colorchooser (GH-6578). (GH-24318) #24325
  • Files
  • tk_rgb.py
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2021-01-25.10:05:25.921>
    created_at = <Date 2018-04-16.21:49:41.715>
    labels = ['type-bug', 'expert-tkinter', '3.10']
    title = 'tkinter askcolor returning floats for r, g, b values instead of ints'
    updated_at = <Date 2021-01-25.10:05:25.920>
    user = 'https://bugs.python.org/BryanOakley'

    bugs.python.org fields:

    activity = <Date 2021-01-25.10:05:25.920>
    actor = 'serhiy.storchaka'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-01-25.10:05:25.921>
    closer = 'serhiy.storchaka'
    components = ['Tkinter']
    creation = <Date 2018-04-16.21:49:41.715>
    creator = 'Bryan.Oakley'
    dependencies = []
    files = ['49756']
    hgrepos = []
    issue_num = 33289
    keywords = ['patch']
    message_count = 14.0
    messages = ['315367', '315548', '315555', '315619', '321551', '321559', '321564', '385400', '385434', '385436', '385438', '385441', '385609', '385612']
    nosy_count = 6.0
    nosy_names = ['terry.reedy', 'vstinner', 'serhiy.storchaka', 'Bryan.Oakley', 'cheryl.sabella', 'miss-islington']
    pr_nums = ['6578', '24318', '24325']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue33289'
    versions = ['Python 3.10']

    @BryanOakley
    Copy link
    Mannequin Author

    BryanOakley mannequin commented Apr 16, 2018

    Even though the underlying tcl/tk interpreter is returning ints, askcolor is converting the values to floats. My guess is this is an oversight related to the change in functionality of the / operator in python3.

    this:

    return (r/256, g/256, b/256), str(result)
    

    should probably be this:

    return (r//256, g//256, b//256), str(result)
    

    @BryanOakley BryanOakley mannequin added 3.7 (EOL) end of life 3.8 only security fixes topic-tkinter labels Apr 16, 2018
    @terryjreedy
    Copy link
    Member

    For future reference, 3.4 and 3.5 only get security fixes.

    The quoted line of code is from the main part of tkinter.colorchooser,Chooser._fixresult:
    r, g, b = widget.winfo_rgb(result)
    return (r/256, g/256, b/256), str(result)

    where tkinter.Misc defines
    def winfo_rgb(self, color):
    """Return tuple of decimal values for red, green, blue for
    COLOR in this widget."""
    return self._getints(
    self.tk.call('winfo', 'rgb', self._w, color))

    The code in tkColorChooser and Tkinter in 2.x is the same.

    The docstring for winfo_rgb is wrong as it returns a tuple of ints in range(2**16256).  For red, the results are
    >>> Tk().winfo_rgb('red')
    (65535, 0, 0)  # 2.x and 3.x
    >>> cc.askcolor('red')
    ((255, 0, 0), '#ff0000')  # 2.7
    >>> cc.askcolor('red')
    ((255.99609375, 0.0, 0.0), '#ff0000')  # 3.8

    In addition to fixing the winfo_rgb docstring (in all versions), and adding doc strings to colorchooser, it seems that '/' should be '//' in 3.x. I don't know if I would fix this in 3.6.

    @terryjreedy terryjreedy changed the title askcolor is returning floats for r,g,b values instead of ints tkinter askcolor returning floats for r,g,b values instead of ints Apr 21, 2018
    @terryjreedy terryjreedy added the type-bug An unexpected behavior, bug, or error label Apr 21, 2018
    @serhiy-storchaka
    Copy link
    Member

    Good catch! A color tuple will likely be used in '#%02x%02x%02x' % color, and this will fail because %x works only with integers (in general sense). Therefore returning a tuple of floats is a bug.

    The downside is that we loss some information. Tk supports up to 16 bit per color component, and askcolor() keeps only higher 8 of them. This can't be changed for backward compatibility, but it may be worth to add an option for control the representation of the result. winfo_rgb() returns 16-bit color components. This is a separate issue.

    @csabella
    Copy link
    Contributor

    Adding to what Serhiy said about the askcolor() return value is an issue that also affects input:

    >>> cc.askcolor('red')
    ((255, 0, 0), '#ff0000')
    >>> cc.askcolor((255, 0, 0))
    ((255, 0, 0), '#ff0000')
    >>> cc.askcolor((65535, 0, 0))
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/home/cheryl/cpython/Lib/tkinter/colorchooser.py", line 75, in askcolor
        return Chooser(**options).show()
      File "/home/cheryl/cpython/Lib/tkinter/commondialog.py", line 43, in show
        s = w.tk.call(self.command, *w._options(self.options))
    _tkinter.TclError: invalid color name "#ffff0000"

    Changing _fixoptions to format initialcolor using "#%04x%04x%04x" % color.

    allows for 16-bit hex values, but it's not the same as 8-bit.

    >>> cc.askcolor('red')
    ((255, 0, 0), '#ff0000')
    >>> cc.askcolor((255, 0, 0))
    ((0, 0, 0), '#000000')
    >>> cc.askcolor((65535, 0, 0))
    ((255, 0, 0), '#ff0000')

    (255, 0, 0) in the second call is black and not red.

    @vstinner
    Copy link
    Member

    Is this issue a regression of Python 3? red/256 gave an integer on Python 2?

    @terryjreedy
    Copy link
    Member

    It appears so.

    @BryanOakley
    Copy link
    Mannequin Author

    BryanOakley mannequin commented Jul 12, 2018

    yes, this is a well known backwards incompatibility. In python 2, the
    division operator returns an integer if both operands are integers. In
    python 3 it returns a float.

    https://www.python.org/dev/peps/pep-0238/

    On Thu, Jul 12, 2018 at 8:48 AM STINNER Victor <report@bugs.python.org>
    wrote:

    STINNER Victor <vstinner@redhat.com> added the comment:

    Is this issue a regression of Python 3? red/256 gave an integer on Python
    2?

    ----------
    nosy: +vstinner


    Python tracker <report@bugs.python.org>
    <https://bugs.python.org/issue33289\>


    @BryanOakley BryanOakley mannequin changed the title tkinter askcolor returning floats for r,g,b values instead of ints tkinter askcolor returning floats for r, g, b values instead of ints Jul 12, 2018
    @zooba zooba removed the easy label Jul 28, 2018
    @terryjreedy
    Copy link
    Member

    https://www.tcl.tk/man/tcl8.6/TkCmd/winfo.htm says
    "winfo rgb window color
    Returns a list containing three decimal values in the range 0 to 65535, which are the red, green, and blue intensities that correspond to color in the window given by window. Color may be specified in any of the forms acceptable for a color option."

    Since tk represents data as strings, 'decimal value in [0, 65535)' means 'integer value represented by decimal digits'. 'correspond to color in the window' implies that the mapping from color to decimal value could depend on the window. The mapping *does* differ between systems (see below).

    https://www.tcl.tk/man/tcl8.6/TkLib/GetColor.htm
    Lists possible return values, hence acceptible forms, as name or '#' followed by 1 to 4 hex digits. It continues with "Each R, G, or B represents a single hexadecimal digit. The four forms permit colors to be specified with 4-bit, 8-bit, 12-bit or 16-bit values. When fewer than 16 bits are provided for each color, they represent the most significant bits of the color, while the lower unfilled bits will be repeatedly replicated from the available higher bits. For example, #3a7 is the same as #3333aaaa7777."

    This omits to say what happens when there are 2 or 3 hex digits. Windows ignores 3rd and 4th hex digits. On Windows, dividing by 257 instead of 256 would be correct. However, 256 works because the 'error', represented by the remainder, is always in [0, 256). MacOS does not ignore digits.

    https://www.tcl.tk/man/tcl8.6/TkCmd/colors.htm
    Lists current color names and their values as 16 bit decimals.

    The PR is blocked because the new colorchooser tests pass on Windows and macOS but fail on Ubuntu. Cheryl speculated that the color names values vary across systems. I think that it might instead be a coding issue, but I want to understand coding on Windows and Mac better before looking into Linux.

    @terryjreedy terryjreedy added 3.10 only security fixes and removed 3.7 (EOL) end of life 3.8 only security fixes labels Jan 21, 2021
    @terryjreedy
    Copy link
    Member

    65535 = 35536 - 1 = 256 * 256 - 1 == 255 * 257

    On Windows, each r, g, b value is n * 257 for n in range(256) (see attached file). The precision loss happens when colors are stored, before the division in winfo_rgb. Perhaps 8 bits/channel (including alpha) is baked in.

    Since divmod(n * 257, 257) = (n, 0) and divmod(n * 257, 256) = (n, n),
    (n*257) // m = divmod(n*257, m)[0] = n whether m is 256 or 257.
    ---

    macOS appears (from limited experiments) to handle 1 and 2 digits the same as Windows (repeat 4 or 2 times): val('#a00') = val('#aaaa00000000') = 0xaaaa, val('#ab0000') = val('#abab00000000') = 0xabab. 4 digits are left alone while 3 digits use the 1st as the 4th val('#abc000000') = val('#abca00000000') = 0xabca.

    @serhiy-storchaka
    Copy link
    Member

    I do not understand why #abc000000 and #abcd00000000 give 0xabab on my computer (Linux) and even weirder result on Ubuntu on CI. Reading the code I expected the same behavior as on macOS.

    @terryjreedy
    Copy link
    Member

    Your Linux result is the same as on Windows. Given strings 'abc' or 'abcd', ignore 'c' or 'cd' and expand 'ab' to 'abab', making value 0xabab. Is your computer Ubuntu (implying that personal Ubuntu != CI Ubuntu) or a different Linux? Are there tk/tcl compilation flags or X window options that could affect stored color values?

    @serhiy-storchaka
    Copy link
    Member

    New changeset 6713e86 by Cheryl Sabella in branch 'master':
    bpo-33289: Return RGB triplet of ints instead of floats from tkinter.colorchooser (GH-6578)
    6713e86

    @serhiy-storchaka
    Copy link
    Member

    New changeset 3d5434d by Serhiy Storchaka in branch '3.9':
    [3.9] bpo-33289: Return RGB triplet of ints instead of floats from tkinter.colorchooser (GH-6578). (GH-24318)
    3d5434d

    @miss-islington
    Copy link
    Contributor

    New changeset 96bcf6f by Miss Islington (bot) in branch '3.8':
    [3.9] bpo-33289: Return RGB triplet of ints instead of floats from tkinter.colorchooser (GH-6578). (GH-24318)
    96bcf6f

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 only security fixes topic-tkinter type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants