colorsys.rgb_to_hls raises ZeroDivisionError on nearly-white inputs

[dfremont]

Bug report

In Python 3.10 and 3.11, the colorsys.rgb_to_hls function divides by zero when given a valid color which is very close to but not exactly pure white.

Python 3.11.1 (main, Feb  8 2023, 15:39:14) [Clang 14.0.0 (clang-1400.0.29.202)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import colorsys
>>> colorsys.rgb_to_hls(1, 1, 0.9999999999999999)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/dfremont/.pyenv/versions/3.11.1/lib/python3.11/colorsys.py", line 86, in rgb_to_hls
    s = rangec / (2.0-sumc)
        ~~~~~~~^~~~~~~~~~~~
ZeroDivisionError: float division by zero

The current code assumes that, given 3 numbers between 0 and 1, if their minimum and maximum are not equal, then the sum of the minimum and maximum must be strictly less than 2. That claim is true for real numbers but not for floating-point arithmetic. The example above fails because 1 + 0.9999999999999999 == 2.0.

Your environment

• CPython versions tested on: 3.8.8 (works); 3.9.13 (works); 3.10.3 (buggy); 3.11.1 (buggy)
• Operating system and architecture: macOS 12.6.6; x86_64

Linked PRs

• gh-106498: Fix an extreme case in colorsys.rgb_to_hls #106530
• gh-106498: Revert incorrect colorsys.rgb_to_hls change #106627
• [3.12] gh-106498: Revert incorrect colorsys.rgb_to_hls change (GH-106627) #106632
• [3.11] gh-106498: Revert incorrect colorsys.rgb_to_hls change (GH-106627) #106633

[gaogaotiantian]

Very interesting and super extreme bug. I don't even know if this should be fixed. In my opinion, "correct" in super extreme case is more important than "very slight performance improvement", but this code has not been touched for a while.

The fix is simple, but will hit performance slightly. We should wait for a core dev to make the decision whether to fix it.

[dfremont]

Thanks, Tian. It's certainly a corner case, but it came up naturally in our project: our code generates random colors, and one of our CI tests failed because we got unlucky and hit this bug. We have to work around it, since it could happen again, so we either need to check for the exception (which is what we're currently doing in BerkeleyLearnVerify/Scenic#153) or implement a correct algorithm ourselves. I'd much rather have the colorsys function be correct but slightly slower.

[terryjreedy]

As @gaogaotiantian noted on PR #106530, the regression is due to issue #86618 and PR #22306, which applied an optimization that was incorrect in one place, by assuming that 2.0 - maxc - minc == 2.0 - (maxc + minc). This is true mathematically, but not for floats in the example case.

>>> 2.0 - 1.0 - 0.9999999999999999
1.1102230246251565e-16
>>> 2.0 - (1.0 + 0.9999999999999999)  # 2.0 - 2.0
0.0

The regression should definitely be fixed, and the correct fix is to undo the incorrect replacement on line 86. The result is 6 different output for the 6 possible cases with maxc = 1.0 and minc = 0.9999999999999999.

rgb                                            hls
1.0, 1.0, 0.9999999999999999                   0.16666666666666666, 1.0, 1.0
1.0, 0.9999999999999999, 1.0                   0.8333333333333334, 1.0, 1.0
0.9999999999999999, 1.0, 1.0                   0.5, 1.0, 1.0
1.0, 0.9999999999999999, 0.9999999999999999    0.0, 1.0, 1.0
0.9999999999999999, 1.0, 0.9999999999999999    0.3333333333333333, 1.0, 1.0
0.9999999999999999, 0.9999999999999999, 1.0    0.6666666666666666, 1.0, 1.0

1.0, 1.0, 1.0                                  0.0, 1.0, 0.0

Replacing the 6 outputs with (0, 1, 0) is incorrect, or at least a change from those prior to #86618. That issue was not intended to make any such change. Note that hls_to_rgb(h, 1, 1) is (1, 1, 1) regardless of h so the outputs above do not roundtrip.

@dfremont I will backport the reversion to 3.11 but cannot do so for 3.10, which now only gets security fixes.