-
Notifications
You must be signed in to change notification settings - Fork 42
Consequent refactor for format and converters independance #31
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
Open
vaab
wants to merge
15
commits into
master
Choose a base branch
from
rc1
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is to ensure minimal dependency towards nosetests and means that even if only the ``colour.py`` is copied in a project, it will be testable with ``python -m doctest colour.py``.
The aim here is to be able to plug formats at will. The Color object will then give easy access to given formats. Thus, it should have no previous knowledge of names of formats nor inner attributes. The remaining attributes will be removed in upcoming commits.
Codecov Report
@@ 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
Continue to review full report at Codecov.
|
API changes: - attribute ``hex_l`` removed in favor of ``hex`` which is now long (6 hexadigit long). Use ``hexs`` for the possibly shortened version to 3 hex digit. - ``LONG_HEX_COLOR`` and ``SHORT_HEX_COLOR`` are not available in module's scope anymore, but are moved in each corresponding format's attribute (``Hex.regex``, and ``HexS.regex``). - ``HEX`` helper class is renamed ``Hex``. - ``xxx2yyy`` conversion functions are unaware of any format and work with basic python types and returns basic python types.
…g with their format name. Color instances would have to infer the target format based with the name of the attribute. For instance ``Color(..).red`` would refer to RGB format only because it is the only format being a ``namedtuple`` AND having a component named 'red'. This behavior is kept if it is not ambiguous, and it is now allowed to be more specific (to avoid ambiguous cases) by prefixing by the format name. For instance, ``.red`` is equivalent to ``.rgb_red``.
API breaks is introduced for all code using ``hue``,``saturation`` to access ``HSL`` components of a ``Color`` object, either as attributes or keyword values when instantiating. Please change these to ``hsl_hue``, and ``hsl_saturation``.
…``.xxx2yyy(..)``. In particular, ``colour.Convertors`` provides all possible conversion methods implemented and integrated in current ``colour`` version.
|
LGTM but AppVeyor configuration does not seems to be working... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
colorsince 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
colorused in a project, I would be very happy and relieved if you could run your tests with this PR (therc1branch) 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:
Engine rewrite to provide easier writing of new formats. [Valentin
Lab]
These are the biggest API changes:
hex_lremoved in favor ofhexwhich is now long (6hexadigit long). Use
hexsfor the possibly shortened version to3 hexadigit when possible.
LONG_HEX_COLORandSHORT_HEX_COLORare not available inmodule's scope anymore, but are moved in each corresponding format's
attribute (
Hex.regex, andHexS.regex).RGB,HEX,HSLhelper classes output now again basicpython types output (tuples and strings). And thus are unaware of
any formats.
xxx2yyyconversion functions are unaware ofany format and work with basic python types and returns basic
python types.
Use namedtuple for rgb and hsl tuples. (fixes Using named tuples #22) [Valentin Lab]
Now let me expand a little on the rewrite part, with this refactor, providing a new format will limit itself to:
Formatobject,Let's illustrate this with the HSL format, here's the defintion of
Hslformat object: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:
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 evenColor("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 ofsetters). Before, colors where stored in HSL format inside the object and so, did need te be converted to this format uponsettercalls.Some advantage:
with a few lines of code.
Colorwith only the format you care about to remove some loadConverter.xxx2yyy(..)methods for all given supported formats.Colorobjects.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.