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: Add *FilmLight E-Gamut 2*. #1323

Merged
merged 6 commits into from
Jan 7, 2025
Merged

Conversation

nick-shaw
Copy link
Contributor

@nick-shaw nick-shaw commented Jan 6, 2025

Summary

Adds the new FilmLight E-Gamut 2 colourspace introduced in Baselight 6. This is a small modification of the original E-Gamut, moving the red and green primaries to enclose AWG4.

Preflight

Code Style and Quality

I apologise that I do not understand the tests sufficiently to run them and fix any errors. GitHub is currently showing CI failures after my commits.

I am happy to be educated on what I should have done!

Documentation

I do not believe any new documentation, other than the included basic docstring is needed

@nick-shaw nick-shaw changed the title E gamut 2 Add FilmLight E-Gamut 2 Jan 6, 2025
@nick-shaw
Copy link
Contributor Author

I believe that adding the lower tolerance for the XYZ matrix unit test means the tests now pass. This was needed because E-Gamut 2 uses an explicitly defined RGB to XYZ matrix, with six decimal places of precision, rather than a matrix derived from the primaries and white point.

@KelSolaar KelSolaar changed the title Add FilmLight E-Gamut 2 PR: Add *FilmLight E-Gamut 2*. Jan 7, 2025
@KelSolaar KelSolaar added this to the v0.4.7 milestone Jan 7, 2025
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.

LGTM, thanks @nick-shaw!

colour/models/rgb/datasets/filmlight.py Outdated Show resolved Hide resolved
@KelSolaar KelSolaar merged commit 9c00f80 into colour-science:develop Jan 7, 2025
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.

2 participants