PR: Implement support for callbacks in colour.continuous.AbstractContinuousFunction class.

[KelSolaar]

Summary

This PR implements support for callbacks in colour.continuous.AbstractContinuousFunction class. This allows concrete child classes to register many callback functions to use when an attribute is set. One use case highlighted in #1120 by @tjdcs is to be able to bypass the dynamic computation of the colour.SpectralDistribution.shape property which is expensive: One can now store the value of the shape during property access and have it reset whenever the underlying colour.Signal._domain attribute is changed.

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.
• [N/A] New transformations have been added to the Automatic Colour Conversion Graph.
• [N/A] 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.

[coveralls]

Coverage: 99.789% (-0.002%) from 99.791% when pulling 09e79b4 on feature/callback into 8e0c9c1 on develop.

[tjdcs]

Reasonable test sets and good first use example. But important architectural changes are requested.

[tjdcs]

Calling every registered call back for every change operation despite the name of the underlying property could cause big problems.

1. very expensive for objects that might have a lot of data and a lot of listeners.
2. callbacks meant to intercept the same property might have an unclear relationship. The value steps should at least chain, so imagine a call back that is meant to capitalize the string of the specified property and another that is meant to append some debug information. In this case only one call back would have the intended side effect
3. this will mix results from callbacks that are meant for different properties

Rather than expect the call back to implement some name validation we should do name validation here. So domain listeners are called when domain is changed.

[tjdcs]

This is a pretty fundamental change to the architecture. I'm wondering two things...

Using @peroperty on a field has lots of nice side effects and lets you set things like @x.setter I wonder if we can highjack the setter decorator, or decorate the setter decorator with something that adds the necissary register call back features on a per-property basis.

An alternative architecture provides a fixed number of "methods" for listening to a property "pre-set" "post_set" would be two ideas (borrowed from Qt's property architecture IIRC, or somewhere else from my UI days), validate the requested property name against existing attributes, and call only the call backs for the specific property at the appropriate time.

[tjdcs]

We can discuss the architectural options on the development hub. The latter is more complicated than necessary but has some nice ideas we can dig into more. The first sounds very pythonic to me, but seems way more difficult.

[KelSolaar]

• very expensive for objects that might have a lot of data and a lot of listeners.

To a degree, you really go through all the callbacks when setting an attribute. I had a version that was actually bound to names with a dict with attribute name as keys and a list of callbacks. It is much more opinionated and it prevent usage of callbacks that acts irrespective of what attribute is changed which is an important use case:

Let's take the colour.Signal function for example, we could handle its invalidation, e.g. https://github.com/colour-science/colour/blob/develop/colour/continuous/signal.py#L355, generically with a single callback that would check if the attribute name is in ("domain", "range", "interpolator", ...) rather than binding a callback for every single name.

The choice is really between something opinionated, restrictive but potentially faster or something without restrictions that could be a bit slower. We should probably spend some time comparing two implementations in term of speed as I would not bet on which one would be the slowest in our context.

callbacks meant to intercept the same property might have an unclear relationship. The value steps should at least chain, so imagine a call back that is meant to capitalize the string of the specified property and another that is meant to append some debug information. In this case only one call back would have the intended side effect

This would happen with any callback / observer pattern, it is user responsibility to make sure that this not happen. In Qt typically, you could have two slots connected to the same signal, one doing something and one reverting it.

this will mix results from callbacks that are meant for different properties

I don't see that necessarily as problem as described in my first paragraph.

Using @peroperty on a field has lots of nice side effects and lets you set things like @x.setter I wonder if we can highjack the setter decorator, or decorate the setter decorator with something that adds the necissary register call back features on a per-property basis.

This will not work because we need to act at the lowest possible level, i.e. attribute, there are plenty of cases where we want to act when a protected attribute i.e. self._domain is changed rather than the self.domain property.

[KelSolaar]

Here is the alternative, opinionated dict alternative implementation: 3e75551

[tjdcs]

The more I think about this, the more I'm convinced that having an attribute name validated implementation is necessary. Consider the case where several attributes are changed at once the list of call backs will all be run several times. But you do make a good case for wanted to be notified when anything about an instance of a class changes. We could have a special attribute name for this purpose. "_on_any_changed"

It's a little bit scary to use a special name without that name actually being a real attribute though...

I just don't like to have so many function calls when there is a lot of attribute changes at once.

[KelSolaar]

Fine by me, I'm happy either way! I'm not too worried as we will hopefully never have to scale that to a lot of calls but I'm fine with the alternative implementation I posted that triggers on attribute name only.

[tjdcs]

Did the alternative implementation already get merged? Looks like it on my commit graph

[KelSolaar]

It seems like I slipped the alternative callback code (3e75551) by mistake with 8fbad05. Let's close this PR as done as it is used by @tjdcs!