vaab / colour

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

new: implement hsv #24

Open JDongian opened 8 years ago

JDongian commented 8 years ago

WIP.

vaab commented 8 years ago

Oh my, I didn't spot these HSV false mentions in the README.rst... thank you for bringing that up.

I've read the rest of the code, and it seems there's more work to do :) . This is a very good start ! Many thanks. Don't forget to document this also in the README.rst, wherever you see hsl you can mention and show hsv examples. And is there's a direct relation between hsl and hsv don't be afraid to show it clearly thanks to a doctest example that will also add a test to this relation. Many thanks for your work !

vaab commented 8 years ago

You might want to read #16

vaab commented 7 years ago

Note: for the final PR, don't forget NOT to commit setup.cfg version information mods ;-).

Tell me when you think it is time for a thorough review on my side.

While I'm at it, note also that what I'll need is also only 1 nice commit that holds doc/tests/implementation (unless you have several features that can be clearly separated by concerns). I guess here 1 commit should be enough. This can be done after my thorough review, or I can do it myself... but then it may delay the inclusion of the code a little bit.

An exception to that if you move code around for code coherence or cosmetics consideration, please separate these in a first commit so that this is very clear that you are only moving code around and it won't create messy diffs for me to review, or for future reading of the changelog history. These commits can be before or after your implementation. (in the current state, your separated commits are a pain to review on github as-is, I need to pull your changes on my side and ask for the whole diff to have a nicer diff...)

If you have trouble using git to clean your code submission, I could do it for you also.