-
Notifications
You must be signed in to change notification settings - Fork 12
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
add CIEDE2000 color distance #14
Conversation
This is looking good. I'll approve when you are ready. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great. Thank you for adding this.
As you said running more/nimpretty and mrdoc will make it 100%.
src/chroma/distance.nim
Outdated
let | ||
C1 = sqrt(c1.a^2 + c1.b^2) | ||
C2 = sqrt(c2.a^2 + c2.b^2) | ||
CM = 0.5*(C1 + C2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0.5*(
I think there are some formatting things that nimpretty will fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was not fixed by nimpretty but I did it manually (I assume you imply spacing the '*')
src/chroma/distance.nim
Outdated
else: | ||
arctan2(x, y).radToDeg + 360 | ||
|
||
func deltaE00*(c1, c2: ColorLAB, k_L, k_C, k_H = 1.float32): float32 = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
k_L
--> kL
Please follow Nim naming convention - no "_".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done. I still have in that file some variable names that start with a Capital letter (although convention would want that only for types). Do you want me to adjust those too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes please.
This reverts commit 98e97f0.
|
I think this looks great merging. |
Hi,
this adds the functionality in https://github.com/pietroppeter/color_distance
I did a bit of cleanup of original code but not too much. Tests work fine.
I still have to add some documentation. I see that you are using mddoc and morepretty, I will try to use that.