Add CLF parsing capabilities.

[MichaelMauderer]

Summary

Adds a module that allows contains the functionality and data structures to parse CLF files.
This is the first step to add full support for CLF into colour.

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.

More docs and examples to come in follow-up PRs, once we can do more than parse the CLF files.

[zachlewis]

Remarkable work -- this is wonderful!

[KelSolaar]

Thanks you! This looks great overall. The main two comments I would have are:

• Maybe considering using an ABC dataclass so that we can consistently expose from_xml and a future to_xml
• I would move all the utility functions to a separate common/utilities module so that the core of the code stays neat and tidy.

[KelSolaar]

Minor

Do we need this function here instead of than calling directly xml.get(attribute_name)?

[KelSolaar]

Minor

At the point we are already leveraging an exception, I would probably remove this clause and use except (TypeError, ValueError): instead.

[KelSolaar]

Pedantry

The quotes are mismatched here:

`style'

[KelSolaar]

Major

Is there a reason for _config arg here with underscore when the other ones are using config, I'm assuming that it is because it is not used?

This makes me think that we should maybe look at an ABC dataclass so that we can enforce the from_xml signature consistently everywhere.

[KelSolaar]

Minor

This definition is not used currently.

[fredsavoir]

Yes, please especially for the separate module. Great works. Fred

…

On 24 Sep 2024, at 09:22, Thomas Mansencal ***@***.***> wrote: @KelSolaar commented on this pull request. Thanks you! This looks great overall. The main two comments I would have are: Maybe considering using an ABC dataclass so that we can consistently expose from_xml and a future to_xml I would move all the utility functions to a separate common/utilities module so that the core of the code stays neat and tidy. In colour/io/clf.py <#1278 (comment)>: > + if config.namespace_name is None: + return name + else: + return f"{{{config.namespace_name}}}{name}" + + +def map_optional(f: Callable, value): + if value is not None: + return f(value) + return None + + +def retrieve_attributes( + xml, attribute_mapping: dict[str, str] +) -> dict[str, str | None]: + def get_attribute(name): Minor Do we need this function here instead of than calling directly xml.get(attribute_name)? In colour/io/clf.py <#1278 (comment)>: > + def get_attribute(name): + return xml.get(name) + + return { + k: get_attribute(attribute_name) + for k, attribute_name in attribute_mapping.items() + } + + +def retrieve_attributes_as_float( + xml, attribute_mapping: dict[str, str] +) -> dict[str, float | None]: + attributes = retrieve_attributes(xml, attribute_mapping) + + def as_float(value): + if value is None: Minor At the point we are already leveraging an exception, I would probably remove this clause and use except (TypeError, ValueError): instead. In colour/io/clf.py <#1278 (comment)>: > + Expects the xml element to be a valid element according to the CLF + specification. + + Raises + ------ + :class: CLFValidationError + If the node does not conform to the specification, a `CLFValidationError` + will be raised. The error message will indicate the details of the issue + that was encountered. + """ + if xml is None: + return None + super_args = ProcessNode.parse_attributes(xml, config) + style = map_optional(ExponentStyle, xml.get("style")) + if style is None: + raise CLFValidationError("Exponent process node has no `style' value.") Pedantry The quotes are mismatched here: `style' In colour/io/clf.py <#1278 (comment)>: > ***@***.*** +class ExponentParams: + """ + Represents a Exponent Params element. + + References + ---------- + https://docs.acescentral.com/specifications/clf/#exponent + """ + + exponent: float + offset: float | None + channel: Channel | None + + @staticmethod + def from_xml(xml, _config: ParserConfig): Major Is there a reason for _config arg here with underscore when the other ones are using config, I'm assuming that it is because it is not used? This makes me think that we should maybe look at an ABC dataclass so that we can enforce the from_xml signature consistently everywhere. In colour/io/clf.py <#1278 (comment)>: > + else: + return xml.xpath(f"{name}/text()") + + +processing_node_constructors = {} + + +def register_process_node_xml_constructor(name): + def register(constructor): + processing_node_constructors[name] = constructor + return constructor + + return register + + +def valid_processing_node_tags(): Minor This definition is not used currently. — Reply to this email directly, view it on GitHub <#1278 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABHZDWXYCFS3H7VLUDNFM6TZYEAJPAVCNFSM6AAAAABKTW6XNOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGMRUGIYDINZUGY>. You are receiving this because you are subscribed to this thread.