Enforce structure of FeatureIdentifier

[tdaniel06]

The documentation indicates that a FeatureIdentifier is a dctionary with a mandatory key named "type". Depending on this type, other keys are also required. In addition, it seems that the value assigned to "type" key must be one of "scalar", "field", "nodes".

For now, the implementation of FeatureIdentifier does not enforce the presence of mandatory keys ("type" and other type-dependent keys) and does not raise any error for unknown types.

Suggestion:

• check structure in constructor, raise errors if unexpected structures ;
• implement setters to raise errors if the user tries to change the object structure with undesired keys after instanciation.

[casenave]

Thank you for this issue: this FeatureIdentifier as dict is kept for backward compatibility, for what will be kept for the V1 is the fact the feature will be identified only by a string corresponding to the CGNS path, even for global from the moment we swapped from csv-based scalar storage to storage in a special base named "Global".

If this answer is OK for you, I can close the issue.

[tdaniel06]

Not sure I understood well. Here is what I would suggest, keeping FeatureIdentifier as dict but adding structure validation:

from typing import Union
import warnings

class FeatureIdentifier(dict[str, Union[str, float]]):
    """Feature identifier for a specific feature."""
    STRUCTURE = {
        'scalar': {
            'required': ['name'],
            'optional': [],
        },
        'field': {
            'required': ['name'],
            'optional': ['base_name', 'zone_name', 'location', 'time'],
        },
        'nodes': {
            'required': ['base_name', 'zone_name', 'time'],
            'optional': [],
        }
    }
    ALLOWED_LOCATIONS = ['Vertex', 'EdgeCenter', 'FaceCenter', 'CellCenter']

    def __init__(self, *args, **kwargs) -> None:
        super().__init__(*args, **kwargs)
        self.__validate(self)
    
    def __validate(self, d: dict[str, Union[str, float]]) -> None:
        if "type" not in d:
            raise KeyError(f"Missing 'type' key in {type(self).__name__}.")
        vtype = d["type"]
        
        if vtype in self.STRUCTURE:
            for r in self.STRUCTURE[vtype]['required']:
                if r not in d:
                    raise ValueError(f"Missing key '{r}', which is mandatory for 'type' '{vtype}'.")
            for k in d:
                if k not in ['type'] + self.STRUCTURE[vtype]['required'] + self.STRUCTURE[vtype]['optional']:
                    warnings.warn(f"Unexpected key '{k}' in {type(self).__name__} for 'type' '{vtype}'.")
            if 'time' in d and not isinstance(d['time'], float):
                raise TypeError("Value for 'time' must be a float.")
            if 'location' in d and d['location'] not in self.ALLOWED_LOCATIONS:
                raise ValueError(f"Invalid location: '{d['location']}', must be one of {self.ALLOWED_LOCATIONS}")
        else:
            raise ValueError(f"Invalid value for 'type': {vtype}. Must be one of {self.STRUCTURE.keys()}")
            
    def __setitem__(self, key: str, value: Union[str, float]) -> None:
        test_dict = {k: self[k] for k in self}
        test_dict[key] = value
        self.__validate(test_dict)
        super().__setitem__(key, value)
    
    def update(self, *args, **kwargs) -> None:
        test_dict = dict(*args, **kwargs)
        test_dict.update(**self)
        self.__validate(test_dict)
        super().update(dict(*args, **kwargs))
            
    def __hash__(self) -> int:  # pyright: ignore[reportIncompatibleVariableOverride]
        """Compute a hash for the feature identifier.

        Returns:
            int: The hash value.
        """
        return hash(frozenset(sorted(self.items())))
        # return hash(tuple(sorted(self.items())))

    def __lt__(self, other: "FeatureIdentifier") -> bool:
        """Compare two feature identifiers for ordering.

        Args:
            other (FeatureIdentifier): The other feature identifier to compare against.

        Returns:
            bool: True if this feature identifier is less than the other, False otherwise.
        """
        return sorted(self.items()) < sorted(other.items())

[casenave]

We've just discussed this IRL, I'm closing