Resolve "Validation of open enumerations does not search for @Custom attribute" (#669)

* search for custom attribute with open enum

* fix tests

* use feature branch from pynxtools-xps

* reorganize error messages

* support custom attribute in validate_nexus

* use pynxtools-xps main branch again

* differntiate between HDF5 and template validation in the case of missing custom attribute

* simplify logic for testing ignored sections

* function docstrings

* add docs for ignore_sections in test framework

Author: lukaspie

docs/how-tos/pynxtools/using-pynxtools-test-framework.md

@@ -63,17 +63,48 @@ def test_foo_reader(nxdl, reader_name, files_or_dir, tmp_path, caplog):
     # of the log files of the reference -nxs file and the one created in the test.
 ```
 
-Alongside the test data in `tests/data`, it is also possible to add other types of test data inside the test directory of the plugin.
+The `ReaderTest.convert_to_nexus` method tries to convert all files in the `files_or_dir` directory to a NeXus file that is compliant with the application definition (`nxdl`), using a specific pynxtools reader (`reader_name`). In this example, the `foo` reader is used to convert to files following the `NXfoo` application definition.
 
-You can also pass additional parameters to `test.convert_to_nexus`:
+There are some possibilities to configure this test for  your specific plugin:
 
-- `caplog_level` (str): This parameter determines the level at which the caplog is set during testing. This can be either "ERROR" (by default) or "WARNING". If it is "WARNING", the test will also fail if any warnings are reported by the reader.
+- You can configure the test data that is used. Typically, this data should be located in `tests/data`, but it is also possible to use other data inside or even outside the test directory of the plugin.
+- You can also pass additional parameters to `test.convert_to_nexus`:
+  - `caplog_level` (str): This parameter determines the level at which the caplog is set during testing. This can be either "ERROR" (by default) or "WARNING". If it is "WARNING", the test will also fail if any warnings are reported by the reader.
 
-- `ignore_undocumented` (boolean): If true, the test skips the verification of undocumented keys. Otherwise, a warning message for undocumented keys is logged.
+  - `ignore_undocumented` (boolean): If true, the test skips the verification of undocumented keys. Otherwise, a warning message for undocumented keys is logged.
+
+Afterwards, the `ReaderTest.convert_to_nexus` method uses the NeXus annotator tool [`read_nexus`](../../learn/pynxtools/nexus-validation.md#read_nexus-nexus-file-reader-and-debugger) (which is part of `pynxtools`) to create log files both of the reference NeXus file located in `files_or_dir` as well as the freshly created NeXus files. These log files are compared line-by-line to check that the created NeXus file is indeed the same as the reference file.
+
+This test can also be configured:
+
+- You can pass a keyword argument `ignore_lines` to `check_reproducibility_of_nexus`. `ignore_lines` is expected to be a list of lines for which the comparison shall be skipped. Specifically, any line that starts with any of the strings in `ignore_lines` is ignored.
+- In adddition, you can disable the comparison for a given line for a NeXus concept in the `read_nexus` output using the `ignore_sections` keyword. As an example, a typical section for a NeXus field in the output looks like this:
+
+    ```
+    DEBUG: 
+    ===== FIELD (//entry/start_time): <HDF5 dataset "start_time": shape (), type "|O">
+    DEBUG: ===== FIELD (//entry/start_time): <HDF5 dataset "start_time": shape (), type "|O">
+    value: 2018-05-01T07:22:00+02:00 
+    DEBUG: value: 2018-05-01T07:22:00+02:00 
+    classpath: ['NXentry', 'NX_DATE_TIME']
+    DEBUG: classpath: ['NXentry', 'NX_DATE_TIME']
+    classes:
+    NXarpes.nxdl.xml:/ENTRY/start_time
+    NXentry.nxdl.xml:/start_time
+    DEBUG: classes:
+    NXarpes.nxdl.xml:/ENTRY/start_time
+    NXentry.nxdl.xml:/start_time
+    <<REQUIRED>>
+    DEBUG: <<REQUIRED>>
+    documentation (NXarpes.nxdl.xml:/ENTRY/start_time):
+    DEBUG: documentation (NXarpes.nxdl.xml:/ENTRY/start_time):
+    ```
+
+    If you do want to disable the comparison for the value of `entry/start_time`, you can pass a dictionary to `ignore_sections`. In this example, the dictionary `{"FIELD (//entry/start_time)": ["value:"]}` would disable the comparison of the `value` line. Any other line in this section can be disabled by adding more strings to the list (e.g. `DEBUG - value:`), whereas additional sections can be ignored by adding to the `ignore_sections` dictionary.
 
 ## How to write an integration test for a NOMAD example in a reader plugin
 
-It is also possible to ship NOMAD Example Uploads directly with the reader plugin. As an example, `pynxtools-mpes` comes with its own NOMAD example (see [here](https://github.com/FAIRmat-NFDI/pynxtools-mpes/tree/bring-in-examples/src/pynxtools_mpes/nomad)) using the `ExampleUploadEntryPoint` of NOMAD (see [here](https://nomad-lab.eu/prod/v1/staging/docs/howto/plugins/example_uploads.html) for more documentation).
+It is also possible to ship NOMAD Example Uploads directly with the reader plugin. As an example, `pynxtools-mpes` comes with its own [NOMAD examplehere](https://github.com/FAIRmat-NFDI/pynxtools-mpes/tree/bring-in-examples/src/pynxtools_mpes/nomad) using the [`ExampleUploadEntryPoint`](https://nomad-lab.eu/prod/v1/staging/docs/howto/plugins/example_uploads.html) of NOMAD.
 
 The `testing` sub-package of `pynxtools` provides two functionalities for testing the `ExampleUploadEntryPoint` defined in a `pynxtools` plugin:
 

src/pynxtools/data/NXtest.nxdl.xml

@@ -99,6 +99,12 @@
                     <item value="1st type open"/>
                     <item value="2nd type open"/>
                 </enumeration>
+                <attribute name="attribute_with_open_enum" optional="true">
+                    <enumeration open="true">
+                        <item value="1st option"/>
+                        <item value="2nd option"/>
+                    </enumeration>
+            </attribute>
             </field>
             <attribute name="group_attribute">
             </attribute>

src/pynxtools/dataconverter/helpers.py

@@ -21,7 +21,7 @@
 import logging
 import os
 import re
-from collections.abc import Mapping, Sequence
+from collections.abc import Mapping, MutableMapping, Sequence
 from datetime import datetime, timezone
 from enum import Enum, auto
 from functools import cache, lru_cache
@@ -87,7 +87,9 @@ class ValidationProblem(Enum):
     UnitWithoutDocumentation = auto()
     InvalidUnit = auto()
     InvalidEnum = auto()
-    OpenEnumWithNewItem = auto()
+    OpenEnumWithCustom = auto()
+    OpenEnumWithCustomFalse = auto()
+    OpenEnumWithMissingCustom = auto()
     MissingRequiredGroup = auto()
     MissingRequiredField = auto()
     MissingRequiredAttribute = auto()
@@ -152,12 +154,27 @@ def _log(self, path: str, log_type: ValidationProblem, value: Optional[Any], *ar
 
         elif log_type == ValidationProblem.InvalidEnum:
             logger.warning(
-                f"The value at {path} should be one of the following: {value}."
+                f"The value '{args[0]}' at {path} should be one of the following: {value}."
             )
-        elif log_type == ValidationProblem.OpenEnumWithNewItem:
+        elif log_type == ValidationProblem.OpenEnumWithCustom:
             logger.info(
-                f"The value at {path} does not match with the enumerated items from the open enumeration: {value}."
+                f"The value '{args[0]}' at {path} does not match with the enumerated items from the open enumeration: {value}."
             )
+        elif log_type == ValidationProblem.OpenEnumWithCustomFalse:
+            logger.warning(
+                f"The value '{args[0]}' at {path} does not match with the enumerated items from the open enumeration: {value}. "
+                "When a different value is used, the boolean 'custom' attribute cannot be False."
+            )
+        elif log_type == ValidationProblem.OpenEnumWithMissingCustom:
+            log_text = (
+                f"The value '{args[0]}' at {path} does not match with the enumerated items from the open enumeration: {value}. "
+                "When a different value is used, a boolean 'custom=True' attribute must be added."
+            )
+            if args[1] is True:
+                log_text += " It was added here automatically."
+                logger.info(log_text)
+            else:
+                logger.warning(log_text)
         elif log_type == ValidationProblem.MissingRequiredGroup:
             logger.warning(f"The required group {path} hasn't been supplied.")
         elif log_type == ValidationProblem.MissingRequiredField:
@@ -287,9 +304,10 @@ def collect_and_log(
         # info messages should not fail validation
         if log_type in (
             ValidationProblem.UnitWithoutDocumentation,
-            ValidationProblem.OpenEnumWithNewItem,
             ValidationProblem.CompressionStrengthZero,
             ValidationProblem.MissingNXclass,
+            ValidationProblem.OpenEnumWithCustom,
+            ValidationProblem.OpenEnumWithMissingCustom,
         ):
             if self.logging and message not in self.data["info"]:
                 self._log(path, log_type, value, *args, **kwargs)
@@ -804,15 +822,11 @@ def convert_int_to_float(value):
         return value
 
 
-def is_valid_data_field(
-    value: Any, nxdl_type: str, nxdl_enum: list, nxdl_enum_open: bool, path: str
-) -> Any:
+def is_valid_data_field(value: Any, nxdl_type: str, path: str) -> Any:
     """Checks whether a given value is valid according to the type defined in the NXDL."""
 
-    def validate_data_value(
-        value: Any, nxdl_type: str, nxdl_enum: list, nxdl_enum_open: bool, path: str
-    ) -> Any:
-        """Validate and possibly convert a primitive value according to NXDL type/enum rules."""
+    def validate_data_value(value: Any, nxdl_type: str, path: str) -> Any:
+        """Validate and possibly convert a primitive value according to NXDL type rules."""
         accepted_types = NEXUS_TO_PYTHON_DATA_TYPES[nxdl_type]
         original_value = value
 
@@ -843,26 +857,6 @@ def validate_data_value(
                     path, ValidationProblem.InvalidDatetime, value
                 )
 
-        if nxdl_enum is not None:
-            if (
-                isinstance(value, np.ndarray)
-                and isinstance(nxdl_enum, list)
-                and isinstance(nxdl_enum[0], list)
-            ):
-                enum_value = list(value)
-            else:
-                enum_value = value
-
-            if enum_value not in nxdl_enum:
-                if nxdl_enum_open:
-                    collector.collect_and_log(
-                        path, ValidationProblem.OpenEnumWithNewItem, nxdl_enum
-                    )
-                else:
-                    collector.collect_and_log(
-                        path, ValidationProblem.InvalidEnum, nxdl_enum
-                    )
-
         return value
 
     if isinstance(value, dict) and set(value.keys()) == {"compress", "strength"}:
@@ -878,18 +872,120 @@ def validate_data_value(
                     path, ValidationProblem.InvalidCompressionStrength, value
                 )
             # In this case, we remove the compression.
-            return validate_data_value(
-                value["compress"], nxdl_type, nxdl_enum, nxdl_enum_open, path
-            )
+            return validate_data_value(value["compress"], nxdl_type, path)
 
         # Apply standard validation to compressed value
-        value["compress"] = validate_data_value(
-            compressed_value, nxdl_type, nxdl_enum, nxdl_enum_open, path
-        )
+        value["compress"] = validate_data_value(compressed_value, nxdl_type, path)
 
         return value
 
-    return validate_data_value(value, nxdl_type, nxdl_enum, nxdl_enum_open, path)
+    return validate_data_value(value, nxdl_type, path)
+
+
+def get_custom_attr_path(path: str) -> str:
+    """
+    Generate the path for the 'custom' attribute for open enumerations for a
+    given path.
+
+    If a NeXus concept has an open enumeration and a different value than the suggested ones are used,
+
+    - for fields, an attribute @custom=True.
+    - for attributes, an additional attribute @my_attribute_custom=True (where my_attribute is the name
+      of the attribute with the open enumeration)
+
+    shall be added to the file. This function creates the path for this custom attribute.
+
+    Args:
+        path (str): The original path string.
+
+    Returns:
+        str: The modified path string representing the custom attribute path.
+    """
+    if path.split("/")[-1].startswith("@"):
+        attr_name = path.split("/")[-1][1:]  # remove "@"
+        return f"{path}_custom"
+    return f"{path}/@custom"
+
+
+def is_valid_enum(
+    value: Any,
+    nxdl_enum: list,
+    nxdl_enum_open: bool,
+    path: str,
+    mapping: MutableMapping,
+):
+    """Validate a value against an NXDL enumeration and handle custom attributes.
+
+    This function checks whether a given value conforms to the specified NXDL
+    enumeration. If the enumeration is open (`nxdl_enum_open`), it may create or
+    check a corresponding custom attribute in the `mapping`.
+
+    Args:
+        value (Any): The value to validate.
+        nxdl_enum (list): The NXDL enumeration to validate against.
+        nxdl_enum_open (bool): Whether the enumeration is open to custom values.
+        path (str): The path of the value in the dataset.
+        mapping (MutableMapping): The object (dict or HDF5 group) holding custom attributes.
+
+    """
+
+    if isinstance(value, dict) and set(value.keys()) == {"compress", "strength"}:
+        value = value["compress"]
+
+    if nxdl_enum is not None:
+        if (
+            isinstance(value, np.ndarray)
+            and isinstance(nxdl_enum, list)
+            and isinstance(nxdl_enum[0], list)
+        ):
+            enum_value = list(value)
+        else:
+            enum_value = value
+
+        if enum_value not in nxdl_enum:
+            if nxdl_enum_open:
+                custom_path = get_custom_attr_path(path)
+
+                if isinstance(mapping, h5py.Group):
+                    parent_path, attr_name = custom_path.rsplit("@", 1)
+                    custom_attr = mapping.get(parent_path).attrs.get(attr_name)
+                    custom_added_auto = False
+                else:
+                    custom_attr = mapping.get(custom_path)
+                    custom_added_auto = True
+
+                if custom_attr == True:  # noqa: E712
+                    collector.collect_and_log(
+                        path,
+                        ValidationProblem.OpenEnumWithCustom,
+                        nxdl_enum,
+                        value,
+                    )
+                elif custom_attr == False:  # noqa: E712
+                    collector.collect_and_log(
+                        path,
+                        ValidationProblem.OpenEnumWithCustomFalse,
+                        nxdl_enum,
+                        value,
+                    )
+
+                elif custom_attr is None:
+                    try:
+                        mapping[custom_path] = True
+                    except ValueError:
+                        # we are in the HDF5 validation, cannot set custom attribute.
+                        pass
+                    collector.collect_and_log(
+                        path,
+                        ValidationProblem.OpenEnumWithMissingCustom,
+                        nxdl_enum,
+                        value,
+                        custom_added_auto,
+                    )
+            else:
+                collector.collect_and_log(
+                    path, ValidationProblem.InvalidEnum, nxdl_enum, value
+                )
 
 
 def split_class_and_name_of(name: str) -> tuple[Optional[str], str]: