Adding the XKCD colour definition to the library

[Gnu-Bricoleur]

Hi, I've been using your module for some time and I was thinking the XKCD colour definition would be a fine addition to it.
https://blog.xkcd.com/2010/05/03/color-survey-results/
This might seem a little bit odd at first as it's not really an accepted standard anywhere but the methodology behind this way of defining colours name is sound and at least one other python library is implementing them (matplotlib). Also, it's fun !
Let me know what you think of it.
I can complete the documentation if you like this pull request.
Also, it might be cleaner to define standard RGB colours in a separate file too so the the architecture is the same for both dictionaries defining colours name but i didn't want to modify too much the library for this first pull request.

[vaab]

Thanks, that was a good read. I'm totally open to integrate xkcd naming into colour. Although, the current proposal in itself could use a little more polishing:

• First, it is far from even working with the current posted code in your current PR, for numerous reasons
• Then, if I like some of your choices, some of them, I'll need to think them through, for instance
  • using uppercase in identifier for xkcd,
  • how to include them : I can't have them pop out in the 'web' color names as these are used to produce
    names in CSS for instance. Not sure that firefox / Chrome are ready for all these identifiers.
• For me, it is a new color naming domain as 'web', 'rgb', 'hsl' ... , 'xkcd' would be a new one.

I would understand that you don't want to push further this PR yourself, and if you don't mind waiting a little, I'll surely integrate this myself, but with a low priority on my list.

After said that, you are welcome to make a better PR if you feel so. I'll send a few remark on your code to give you more clues about what to improve.

[vaab]

Thanks for the pointer !

[vaab]

avoir uppercases, and import *, in general, I'm not sure this requires to be in another file.

[Gnu-Bricoleur]

I wasn't aware of this good practice, but i'll keep it in mind !
I put it in another file because i wasn't sure adding a thousand line of hard-coded constant would be good for readability, but it can totally be included in the main file !

[vaab]

Well, I can't really see how this even runs : COLOR_NAME_TO_RGB is not defined at this point, and is defined the next instruction. Please also notice that the format you use is closer to RGB_TO_COLOR_NAME but a simple update anyway will not make it : the key is meant to be 3-uple of RGB integers... not floats. And the update will overwrite previous names instead of appending them upon key collision which I expect to happen.

[Gnu-Bricoleur]

Yes, that's totally correct, the line should be :
RGB_TO_COLOR_NAME.update(XKCD_RGB_TO_COLOR_NAMES)
I messed the pull request as i originally wrote the code in another folder and then copied it manually in a newly cloned repository to make the pull request, i though i could manage it as it's only one line but apparently not 😅
I though it was working because it wasn't throwing errors but you're right, it's unclean and probably overwriting keys, i'll look into a better solution !

[vaab]

You could totally use these values, but it seems that original content from xkcd defines also RGB 3-uple integer, or at least standard hexa counterparts (I'm refering to https://xkcd.com/color/rgb.txt ). It seems to be a much more solid definition than a float approximation.

[vaab]

Let's add also that we don't want to mix 'web' format with 'xkcd' one, and the current structure of colour is quite friendly to add new formats. So why not use a similar table without the XKCD_ prefix. We would then wait for an attribute Colour().xkcd that could be read or written to, and a instanciation time keyword Colour(xkcd='light eggplant'). You could actually start the xkcd realm as an exact copy of web realm, but using simply another correspondance table... This would also give you the defaulting of using hexa decimal forms when no xkcd definition exists.

[Gnu-Bricoleur]

Whooo, first of all, thanks a lot for taking the time to review my request
I really appreciate your feedback and I'm going to take the opportunity and try to make a good pull request !
I'm going to look through your comments and correct everything

[vaab]

Don't hesitate to push unfinished code in this PR to ask for confirmation on some of your choices, this could help you not loosing to much time on things I might refuse. You may probably also want to start by the documentation so as to make sure that we have the same view on what is the objective for the final user.

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (master@11f138e). Click here to learn what that means.
The diff coverage is 75.00%.

@@            Coverage Diff            @@
##             master      #55   +/-   ##
=========================================
  Coverage          ?   98.72%           
=========================================
  Files             ?        1           
  Lines             ?      236           
  Branches          ?        0           
=========================================
  Hits              ?      233           
  Misses            ?        3           
  Partials          ?        0           

Impacted Files	Coverage Δ
colour.py	98.72% <75.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 11f138e...635483e. Read the comment docs.

[Gnu-Bricoleur]

Hi Valentin,

So I tried to take into account your remarks and include the XKCD naming scheme in a similar fashion to the web color.
It's still incomplete and it's not possible to get the xkcd name from a color defined with hex values. However, the basics works => defining a color with a xkcd code name and it's possible to use it as a normal color object afterwards.
c = Color(xkcd="baby_blue")
What do you think of it ?
Is it going in the right direction according to you ?

[Gnu-Bricoleur]

Hi Valentin,

I just wanted to check with you if you had the time to get a look at the modifications I made to the request?

Regards,
Sylvain