The colorsys library does not follow cpython implementation #28
Comments
|
Thanks for raising the issue. Looking back at the original commits for this library it looks like it was originally taken from the cpython implementation and then with the next commit changed to 0-255 ints: It's unclear why that change was made, perhaps the original authors intended use needed the 0-255 int colors. Adding to the confusion the documented return types show float when in actuality the current version returns ints as noted. https://docs.circuitpython.org/projects/colorsys/en/latest/api.html#colorsys.hls_to_rgb Instead of adding a warning in the implementation notes, I'd be in favor of changing the functionality to match cpython and return floats instead. If application code needs int colors they could convert it outside of this module so that we can keep compatibility to a maximum where possible. |
|
Hello, I didn’t propose a change in the library itself because it would break the existing code using it. ( I can’t resist to insert an xkcd reference here ) However, this is my personnal point of view, and if you are willing to change the behavior itself, please do :) |
|
A classic :) That is true, and I'll definitely get input from the wider team. I had a look around for usages in the most prominent places as well. In all of the Learn Guide repo, as well as all Library examples within the bundle there are two usages of
Those were the only two I found in those places searching on "colorsys" string. |
Hello,
I’m openning this issue because I didn’t found any mention of this in the documentation in the page implementation-notes from the library.
There is an existing library colorsys in cpython with the methods named
hls_to_rgborhsv_to_rgband the developper might be tempted to believe that the code running in cpython could run in circuitpython in the same way.This is wrong : the functions in cpython returns a float with a range from 0…1 where circuitPython return an int between 0…255.
Could you at least add a caveat in the documentation in order to rise the attention on this point ?
The text was updated successfully, but these errors were encountered: