PR: Implement Support for Executing CLF Workflows

[MichaelMauderer]

Summary

Add support for executing CLF workflows defined as per https://docs.acescentral.com/specifications/clf/#common-lut-format-clf-a-common-file-format-for-look-up-tables

Closes #636

TODO

• Extend documentation
• Add tests for more complex workflows
• Add better error handling for invalid inputs
• Create a release of the CLF IO package and update the pyproject.tomlto use that instead of directly referring to the repo.

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.

[KelSolaar]

Thanks Michael for the work! This is just a first batch of important notes WRT to the implementation, core idea is that we try as much as possible to use our existing code.

[KelSolaar]

Major

This should be an optional dependency as we do not want to hard-depend on CLF.

We will thus need a new requirement entry and callable here: https://github.com/colour-science/colour/blob/develop/colour/utilities/requirements.py

We will also want to track its version in colour.utilities.describe_environment

[MichaelMauderer]

This needs to be done after the clf_io package is released.

[MichaelMauderer]

Done.

[KelSolaar]

Minor

This is fine for now but we will need to release it on Pypi.

[MichaelMauderer]

Definitely. Just wanted to wait until everything else is ironed out.

[MichaelMauderer]

Done.

[KelSolaar]

Minor

Do you have the precommit hooks running, it looks like isort is not really running.

[MichaelMauderer]

They seem to be running without error on my machine at the moment.

[KelSolaar]

Major

We should try to use colour.io.LUTSequence here.

[MichaelMauderer]

Done.

[coveralls]

coverage: 97.381% (-0.1%) from 97.478%
when pulling d829370 on feature/CLF_execution
into 6d73ea8 on develop.

[KelSolaar]

Thanks @MichaelMauderer! Not a full review yet because I would like to tame some high level issues first before going deeper.

Singleton Behaviour

Was there any reason to have a node class global attribute here:

class CLFNode(AbstractLUTSequenceOperator):
    node: clf.ProcessNode

    def __init__(self, node: clf.ProcessNode) -> None:
        super().__init__(node.name, node.description)
        self.node = node

The consequence is that if we instantiate multiple CLF nodes of the same type, they behave like a singleton and changing one will change all the others:

I think that this would be better:

class CLFNode(AbstractLUTSequenceOperator):
    def __init__(self, node: clf.ProcessNode) -> None:
        super().__init__(node.name, node.description)
        self._node = node

    @property
    def node(self) -> clf.ProcessNode:


class LUT3D(CLFNode):
    def __init__(self, node: clf.LUT3D) -> None:
        super().__init__(node)
        # self.node = node  <--- We don't even need to assign it here because it is taken care of in the super class.

The classes from colour_clf_io are fine because they are @dataclass decorated:

import colour_clf_io as clf

LUT_1 = clf.LUT1D("Foo", "Bar", clf.BitDepth.f32, clf.BitDepth.f32, "Baz", clf.Array([0, 1, 2, 3, 4], [0, 1, 2, 3, 4]), False, False, clf.Interpolation1D.LINEAR)
LUT_2 = clf.LUT1D("John", "Doe", clf.BitDepth.f16, clf.BitDepth.f16, "Luke", clf.Array([4, 3, 2, 1, 0], [4, 3, 2, 1, 0]), True, True, clf.Interpolation1D.LINEAR)

print(LUT_1.array)
print(LUT_2.array)

Array(values=[0, 1, 2, 3, 4], dim=[0, 1, 2, 3, 4])
Array(values=[4, 3, 2, 1, 0], dim=[4, 3, 2, 1, 0])

BTW, I noted that colour_clf_io .XMLParsable already inherits from ABC thus we can simplify colour_clf_io.ProcessNode inheritance and remove ABC:

@dataclass
class ProcessNode(XMLParsable, ABC)

apply

Instead of having an apply function, why don't we override LUTSequence.__init__ so that usage becomes something along those lines which is consistent with the way we apply LUTs, etc...:

class CLFProcessList(LUTSequence):
     def __init__(process_list: clf.ProcessList):
          ...

my_CLF = CLFProcessList(process_list)
RGB = my_CLF.apply(RGB)

[MichaelMauderer]

Singleton Behaviour

...

Thanks for spotting this! This bit was a bit tricky due to typing requirements. We want the CLFNode node to have access to a clf.ProcessNode, but each subclass needs to have access to the actual subclass of clf.ProcessNode that it's based on, so it can access the specific attributes. Unfortunately, the proposed solution doesn't allow for that, as it erases the specific type of the node used. The solution I've arrived at is a bit more verbose, but seems to work fine:

class CLFNode(AbstractLUTSequenceOperator):
    def __init__(self, node: clf.ProcessNode) -> None:
        super().__init__(node.name, node.description)

    @property
    @abstractmethod
    def node(self) -> clf.ProcessNode:

class LUT3D(CLFNode):
   
    def __init__(self, node: clf.LUT3D) -> None:
        super().__init__(node)
        self._node = node

    @property
    def node(self) -> clf.LUT3D:
        return self._node

This passes type checking and avoids the singleton bug.

apply

...

I agree that is a much better API. I went with the apply approach to keep the colour_clf_io API out of the public API of colour for stability reasons. But as discussed, we could just move this whole PR to colour_clf_io and avoid the issue completely.
There we can add the CLFProcessList you propose here behind an optional feature that adds colour as a dependency to colour_clf_io.