vaab / colour

Python color representations manipulation library (RGB, HSL, web, ...)
BSD 2-Clause "Simplified" License
319 stars 41 forks source link

Consequent refactor for format and converters independance #31

Open vaab opened 7 years ago

vaab commented 7 years ago

I'm Valentin Lab, the original author of python package 'colour'. I'm pinging @multani @priestc @hellais @mehcode @JDongian because you have contributed in one way or another to 'colour' package. So first of all : thank you all.

I have just rewritten a substantial part of 'colour' without changing the tests. This should somewhat garantee some level of iso-functionality. But I would hate breaking anything in your possible usage of 'color'.

I have committed this on github in this PR.

I guess some of you have moved away from color since a long time, or others simply don't have time to help me now. Please then ignore this PR and emails (and you can unregiser I guess from this thread), and thank you again for your help last time. It was and is still very appreciated.

For those that currently have color used in a project, I would be very happy and relieved if you could run your tests with this PR (the rc1 branch) to check that nothing odd is happening as a consequence of this rewrite.

For the one interested, and for documentation purpose, I'll now develop some aspect of what changed, and why:

Let me first start with the changelog for end-users:

Now let me expand a little on the rewrite part, with this refactor, providing a new format will limit itself to:

Let's illustrate this with the HSL format, here's the defintion of Hsl format object:

  @register_format(Formats)
  class HSL(Tuple("hue", "saturation", "luminance")):
      """3-uple of Hue, Saturation, Lightness all between 0.0 and 1.0"""

Yes, that's all. We can easily see that providing any CMYK, HSV, YUV Format object should not be so difficult.

What you need then is to provide at least a way to convert to any format already supported in your format registry, back and forth. Here's how it is done:

    @register_converter(Converters, HSL, RGB)
    def hsl2rgb(hsl):
        ...

    @register_converter(Converters, RGB, HSL)
    def rgb2hsl(rgb):
        ...

The color object will figure itself all the rest... getter/setters on the different format available like Color("red").rgb ... using format names as attribute. Or even Color("red").saturation ... using name of subattributes of a format.

There are still some design choice that could change. And in this new version of Color, now stores the in a variable format (you can fix it if needed) and it will change to the last valuation (usage of setters). Before, colors where stored in HSL format inside the object and so, did need te be converted to this format upon setter calls.

Some advantage:

I'll probably add an additional commit with a YUV, HSV, or CMYK additional format to check that everything is okay.

Of course, if you feel like reviewing some code, that will be appreciated also. But what I need first is checking that I don't break anything.

codecov[bot] commented 7 years ago

Codecov Report

Merging #31 into master will decrease coverage by 0.12%. The diff coverage is 98.79%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #31      +/-   ##
==========================================
- Coverage   99.13%   99.01%   -0.13%     
==========================================
  Files           1        1              
  Lines         232      405     +173     
  Branches        0      121     +121     
==========================================
+ Hits          230      401     +171     
+ Misses          2        0       -2     
- Partials        0        4       +4
Impacted Files Coverage Δ
colour.py 99.01% <98.79%> (-0.13%) :arrow_down:

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 c64602b...870dc94. Read the comment docs.

pradyunsg commented 7 years ago

LGTM but AppVeyor configuration does not seems to be working...