Skip to content

bpo-45775: Implement rgb_to_yuv and yuv_to_rgb for colorsys #29512

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

Closed
wants to merge 6 commits into from

Conversation

tstolarski
Copy link

@tstolarski tstolarski commented Nov 10, 2021

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the many updates ;-) Your PR not only add 2 new functions with tests, but also enhance the documentation and tests of existing functions!

I would appreciate a review from another core dev since I'm not used to the colorsys module.

cc @serhiy-storchaka

these color spaces are floating point values. In the YIQ and YUV space, the
Y coordinate is between 0 and 1, but the I, Q, U and V coordinates can be
positive or negative. In all other spaces, the coordinates are all between
0 and 1.
Copy link
Member

Choose a reason for hiding this comment

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

Should we note what’s the range for I, Q, U, V too or is it obvious if you’re familiar with YIQ/YUV?

Copy link
Author

Choose a reason for hiding this comment

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

From the BT.709 standard, U should be [-0.436...0.436], and V should be [-0.615...0.615], but since YIQ is using the FCC NTSC, its range in float terms (as opposed to wavelength terms) doesn't seem to be well documented (but it seems to be roughly [-0.5984...0.5984] for I, and [-0.5246..0.5246] for Q).

Since it's kind of varied I figure it's probably best to not bulk these in with this section, but maybe they belong somewhere else (e.g. with the methods?), or if there's a reliable document with these values it might be worth linking to that instead.

@encukou encukou self-requested a review November 18, 2021 15:12
@vstinner
Copy link
Member

The https://bugs.python.org/issue45775 has been rejected.

You can write your own module on PyPI with more advanced features if you want to extend colorsys ;-)

@vstinner vstinner closed this Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants