-
Notifications
You must be signed in to change notification settings - Fork 5
FIx a bug to add missing BlackBody & Daylight Options #130
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
base: master
Are you sure you want to change the base?
Conversation
… daylight with custom temperatures These need to be passed through and handled in the backend to ensure we get the right illuminant We also have to update the cache_Key to invalidate the cache if the lluminant changes other wise we run with the wrong settings
| raise ValueError(msg) | ||
| # Apply the correction factor as per CIE 015:2004 recommendation | ||
| CCT_corrected = temperature * 1.4388 / 1.4380 | ||
| xy = CCT_to_xy_CIE_D(CCT_corrected) |
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.
@KelSolaar thomas sir would be great if you can double check for me this is the right way to handle black body and daylight options.
|
There is something quite not right about this as the UI does already some of that work (the function is actually duplicated already), we should probably consolidate this so that there is only one entry point to generate the illuminants. Thoughts? |
|
Hey @KelSolaar 100% And exactly the kind of info i was looking from you! In general there seems to be a fair amount of logic remianing in the UI which we should move either to the common helper functions, or the application itself that the UI then only has to deal with updating the display vs processing any data. |
|
Ill try and make a stab at this change later in the week |
The Daylight and Blackbody options have been used by folks in production who reported it not working.
Needed to add handling to pass the custom temperature through to the backend
Needed to add in the code to calculate and get the illuminent for those two options