Skip to content
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

PR: Implement support for *HyAB* large color difference. #1305

Merged
merged 8 commits into from
Nov 1, 2024

Conversation

mesvam
Copy link
Contributor

@mesvam mesvam commented Oct 24, 2024

Summary

Implement HyAB color difference and resolves #610

Preflight

Code Style and Quality

  • Unit tests have been implemented and passed.
  • Pyright static checking has been run and passed.
  • Pre-commit hooks have been run and passed.
  • New transformations have been added to the Automatic Colour Conversion Graph.
  • New transformations have been exported to the relevant namespaces, e.g. colour, colour.models.

Documentation

  • New features are documented along with examples if relevant.
  • The documentation is Sphinx and numpydoc compliant.

@mesvam
Copy link
Contributor Author

mesvam commented Oct 24, 2024

I think I got the implementation correct, but could somebody double check the other stuff: documentation, tests, citation, formatting, etc.? I'm not sure I got all the tests to run correctly.

@KelSolaar KelSolaar changed the base branch from develop to feature/hyab_staging October 28, 2024 07:15
Copy link
Member

@KelSolaar KelSolaar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @mesvam,

I added a few important comments, what about also adding cbLAB? Would be great!

Cheers,

Thomas

colour/difference/delta_e.py Outdated Show resolved Hide resolved
colour/difference/delta_e.py Outdated Show resolved Hide resolved
colour/difference/delta_e.py Outdated Show resolved Hide resolved
@mesvam
Copy link
Contributor Author

mesvam commented Oct 28, 2024

I just noticed there are functions for manhattan_distance and euclidean_distance in colour.algebra. Would you prefer I use those functions to implement these difference metrics instead?

@mesvam
Copy link
Contributor Author

mesvam commented Oct 28, 2024

what about also adding cbLAB? Would be great!

I don't mind adding cbLAB if you like, as well as some of the other metrics in that paper, but the authors recommended HyAB and HyCH as the top candidates. We can also add all four formulas, but maybe that's too much or too confusing.

Also, would you prefer that in a separate PR, or just tack it onto this one?

@mesvam
Copy link
Contributor Author

mesvam commented Oct 29, 2024

Ok, I'm stumped. How do I pass Codacy? I can't seem to find the error messages

@KelSolaar KelSolaar changed the title add HyAB color difference PR: Implement support for *HyAB* large color difference. Oct 29, 2024
@KelSolaar
Copy link
Member

I just noticed there are functions for manhattan_distance and euclidean_distance in colour.algebra. Would you prefer I use those functions to implement these difference metrics instead?

If you can wherever possible, this would be fantastic!

I would probably try to get HyCH actually if you please don't mind:

The HyAB has the best consistency with CIELAB for
all categories of the second dataset. The HyCH has a good
consistency with CIEDE2000 for the second dataset as
well. So, the HyAB and HyCH could be used for small to
large color difference calculation. However, the HyAB
has a slightly better performance and the advantage of
simpler computation.
It is thus concluded that, for very large color differ-
ences, the effect of lightness difference should be consid-
ered perceptually separable, and the HyAB color
difference formula is the most practical and accurate
solution.

Ok, I'm stumped. How do I pass Codacy? I can't seem to find the error messages

Do not worry, it is all good atm!

@mesvam
Copy link
Contributor Author

mesvam commented Oct 30, 2024

I submitted a separate PR for HyCH #1307

@mesvam
Copy link
Contributor Author

mesvam commented Oct 30, 2024

On second thought, I think using manhattan_distance and euclidean_distance would just slow it down and make it more complicated. I'm pretty happy with this as is right now.

@mesvam mesvam requested a review from KelSolaar October 30, 2024 15:04
@KelSolaar KelSolaar changed the base branch from feature/hyab_staging to develop November 1, 2024 19:58
Copy link
Member

@KelSolaar KelSolaar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Merging this one!

@KelSolaar KelSolaar added this to the v0.4.7 milestone Nov 1, 2024
@KelSolaar KelSolaar merged commit 363145a into colour-science:develop Nov 1, 2024
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement support for "HyAB" large colour difference metric.
2 participants