search

colour-science / colour

Colour Science for Python
https://www.colour-science.org
BSD 3-Clause "New" or "Revised" License
2.08k stars 259 forks source link

Issue in implementation of "Finlayson et al. (2015)" method. #604

Closed ChunHsinWang closed 4 years ago

ChunHsinWang commented 4 years ago

The function def polynomial_expansion_Finlayson2015() for polynomial expansion for Finlayson et al. (2015) method seems to be implemented incorrectly when root_polynomial_expansion is set to True.
https://github.com/colour-science/colour/blob/c3245b1830323991823f20580f81eee856dcc04c/colour/characterisation/correction.py#L258 For instance, the term (R * G) * 1 / 2 where (RG) is supposedly taking a square root is falsely calculated as to the power of 1 and divided by 2. An easy fix would be adding brackets to all nth root terms.

KelSolaar commented 4 years ago

Hi @ChunHsinWang,

Thanks! Great catch, embarrassing but should be fixed soon!

KelSolaar commented 4 years ago

It should be fixed @ChunHsinWang! :)

rsnitsch commented 4 years ago

I would like to use the Finlayson method in production. When will 0.3.16 with the fix be released?

KelSolaar commented 4 years ago

Hi rsnitch,

The original plan was to do it after we have integrated all the GSoC code, so sometimes September, but highly depends on my load (which has been insanely high lately).

Cheers,

Thomas

rsnitsch commented 4 years ago

Could you maybe publish a hotfix version 0.3.16 with the bugfix (possibly including some other bugfixes, too) and move the other changes to 0.3.17? That would be awesome.

Otherwise I'll probably have to try and specify the repository+commit directly in the Pipfile. Or I'll incorporate a copy of the fixed code into my own code for the time being.

KelSolaar commented 4 years ago

No sorry, I don't have the free cycles to go through a full Colour release, with the amount of changes we have added compared to 0.3.15, the CI being broken, etc... it will take a little while, not only that but I will need to bubble up some of the dependent repositories.

I would indeed recommend that either you pull down from the repo if you can OR monkey patch directly in your code indeed.