Skip to content

gh-124879: simplify colorsys.rgb_to_hsv #124880

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

gh-124879: simplify colorsys.rgb_to_hsv #124880

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0894268
inline variables, combining divisions
Yay295 Oct 2, 2024
6bb2bbe
📜🤖 Added by blurb_it.
blurb-it[bot] Oct 2, 2024
c48c44b
Update Misc/NEWS.d/next/Library/2024-10-02-16-47-45.gh-issue-124879.9…
Yay295 Oct 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions Lib/colorsys.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,15 +130,12 @@ def rgb_to_hsv(r, g, b):
if minc == maxc:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the function become faster for greyscale values if rangec = (maxc-minc) is moved after the if check?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

surprisingly, no
Old: 6.133919600164518
New: 6.310812799958512

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes:

Old: 4.684144808999999
New: 4.411844383999999

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange, could you try running this?

import timeit

def rgb_to_hsv_old(r, g, b):
    maxc = max(r, g, b)
    minc = min(r, g, b)
    rangec = (maxc-minc)
    v = maxc
    if minc == maxc:
        return 0.0, 0.0, v
    s = rangec / maxc
    if r == maxc:
        h = (g-b) / rangec
    elif g == maxc:
        h = 2.0 + (b-r) / rangec
    else:
        h = 4.0 + (r-g) / rangec
    h = (h/6.0) % 1.0
    return h, s, v

def rgb_to_hsv_new(r, g, b):
    maxc = max(r, g, b)
    minc = min(r, g, b)
    v = maxc
    if minc == maxc:
        return 0.0, 0.0, v
    rangec = (maxc-minc)
    s = rangec / maxc
    if r == maxc:
        h = (g-b) / rangec
    elif g == maxc:
        h = 2.0 + (b-r) / rangec
    else:
        h = 4.0 + (r-g) / rangec
    h = (h/6.0) % 1.0
    return h, s, v

print('Old:',timeit.timeit(lambda: rgb_to_hsv_old(9,9,9),number=10_000_000))
print('New:',timeit.timeit(lambda: rgb_to_hsv_new(9,9,9),number=10_000_000))

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Old: 4.841873000143096
New: 4.7804170998279005

The arguments to rgb_to_hsv are floats in the range [0,1] though, not integers.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah my bad, try with 0.5 then.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Old: 4.947571000084281
New: 4.570939600002021

though with (0.2,0.5,0.7) I get

Old: 6.850879299920052
New: 6.9389673001132905

so it is faster when the numbers are the same, but slower when they aren't

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it's just random:

Old: 5.283142290998512
New: 5.288310900999932

return 0.0, 0.0, v
s = rangec / maxc
rc = (maxc-r) / rangec
gc = (maxc-g) / rangec
bc = (maxc-b) / rangec
if r == maxc:
h = bc-gc
h = (g-b) / rangec
elif g == maxc:
h = 2.0+rc-bc
h = 2.0 + (b-r) / rangec
else:
h = 4.0+gc-rc
h = 4.0 + (r-g) / rangec
h = (h/6.0) % 1.0
return h, s, v

Expand Down
Loading