classification
Title: colorsys rgb_to_hls algorithm error
Type: Stage: resolved
Components: Library (Lib) Versions:
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: Jim.Jewett, Kunal Grover, Mats Luspa, The_Knight, aarqon, serhiy.storchaka, vstinner, yselivanov
Priority: normal Keywords: easy

Created on 2016-02-05 14:00 by Mats Luspa, last changed 2017-01-14 14:52 by serhiy.storchaka. This issue is now closed.

Messages (8)
msg259659 - (view) Author: Mats Luspa (Mats Luspa) Date: 2016-02-05 14:00
In the colorsys library function rgb_to_hls the algorithm is not implemented quite correctly.

According to algorithm the correct implementation should be (the error is in the condition r == maxc). In the current code g<b is not tested:

def rgb_to_hls(r, g, b):
    maxc = max(r, g, b)
    minc = min(r, g, b)
    # XXX Can optimize (maxc+minc) and (maxc-minc)
    l = (minc+maxc)/2.0
    if minc == maxc:
        return 0.0, l, 0.0
    if l <= 0.5:
        s = (maxc-minc) / (maxc+minc)
    else:
        s = (maxc-minc) / (2.0-maxc-minc)
    rc = (maxc-r) / (maxc-minc)
    gc = (maxc-g) / (maxc-minc)
    bc = (maxc-b) / (maxc-minc)
    if r == maxc:
        if (g<b):
            h = bc-gc+6
        else:
            h = bc-gc
    elif g == maxc:
        h = 2.0+rc-bc
    else:
        h = 4.0+gc-rc
    h = (h/6.0) % 1.0
    return h, l, s
msg259660 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-02-05 14:02
Oh, someone uses colorsys :-)
msg259686 - (view) Author: Shashank Agarwal (The_Knight) Date: 2016-02-05 19:23
I would like to work on this..
msg259830 - (view) Author: Kunal Grover (Kunal Grover) Date: 2016-02-08 09:05
This isn't needed. We are taking modulus with 1.0, which already fixes negative values of h.
msg260062 - (view) Author: Jim Jewett (Jim.Jewett) * (Python triager) Date: 2016-02-11 00:13
The modulus fixes it for exact numbers.  It doesn't produce exactly the same result with floats, because of rounding.  Is that a problem?
msg285453 - (view) Author: Bryan B (aarqon) Date: 2017-01-14 01:58
Adding myself to this since I'm going to fix another hiccup in this file and I might as well clean this up too.
msg285455 - (view) Author: Bryan B (aarqon) Date: 2017-01-14 02:19
Well, the other issue was resolved by updating Python on my computer to 3.6 ;)

Setting up the entire Python build and test environment for an issue this small seems a little excessive, especially for a module that seems seldomly used. I'm gonna have to be that guy and let this one sit as well.

Sorry :(
msg285485 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-01-14 14:52
> The modulus fixes it for exact numbers.  It doesn't produce exactly the same result with floats, because of rounding.  Is that a problem?

I don't think that is a problem. The function uses a number of floating point operations producing inexact intermediate values. There is not an evidence that the proposed code produces more accurate value.
History
Date User Action Args
2017-01-14 14:52:12serhiy.storchakasetstatus: open -> closed

nosy: + serhiy.storchaka
messages: + msg285485

resolution: not a bug
stage: needs patch -> resolved
2017-01-14 02:19:10aarqonsetmessages: + msg285455
title: colorys rgb_to_hls algorithm error -> colorsys rgb_to_hls algorithm error
2017-01-14 01:58:00aarqonsetnosy: + aarqon
messages: + msg285453
2016-02-11 00:13:41Jim.Jewettsetnosy: + Jim.Jewett
messages: + msg260062
2016-02-08 09:05:27Kunal Groversetnosy: + Kunal Grover
messages: + msg259830
2016-02-05 19:23:01The_Knightsetnosy: + The_Knight
messages: + msg259686
2016-02-05 14:33:48serhiy.storchakasetkeywords: + easy
stage: needs patch
2016-02-05 14:02:07vstinnersetnosy: + vstinner, yselivanov
messages: + msg259660
2016-02-05 14:00:12Mats Luspacreate