From 0f46c34200bfe2a748cad9bfc273cd4ffdffa606 Mon Sep 17 00:00:00 2001 From: Rubel Date: Tue, 18 Feb 2025 15:11:26 +0100 Subject: [PATCH 01/36] Fix the nx_char type for numpy to and . --- src/pynxtools/dataconverter/helpers.py | 5 +++-- tests/dataconverter/test_helpers.py | 18 +++++++++--------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/pynxtools/dataconverter/helpers.py b/src/pynxtools/dataconverter/helpers.py index 71d4a4b9f..b9e99d1c6 100644 --- a/src/pynxtools/dataconverter/helpers.py +++ b/src/pynxtools/dataconverter/helpers.py @@ -591,12 +591,13 @@ def is_value_valid_element_of_enum(value, elist) -> Tuple[bool, list]: np.uint16, np.uint32, np.uint64, + np.uint, np.unsignedinteger, np.signedinteger, ) np_float = (np.float16, np.float32, np.float64, np.floating) np_bytes = (np.bytes_, np.byte, np.ubyte) -np_char = (np.str_, np.char.chararray, *np_bytes) +np_char = (np.str_, np.bytes_) # Only numpy Unicode string and Byte string np_bool = (np.bool_,) np_complex = (np.complex64, np.complex128, np.cdouble, np.csingle) NEXUS_TO_PYTHON_DATA_TYPES = { @@ -608,7 +609,7 @@ def is_value_valid_element_of_enum(value, elist) -> Tuple[bool, list]: *np_bytes, ), "NX_BOOLEAN": (bool, np.ndarray, *np_bool), - "NX_CHAR": (str, np.ndarray, *np_char), + "NX_CHAR": (str, *np_char), "NX_DATE_TIME": (str,), "NX_FLOAT": (float, np.ndarray, *np_float), "NX_INT": (int, np.ndarray, *np_int), diff --git a/tests/dataconverter/test_helpers.py b/tests/dataconverter/test_helpers.py index 0ef64ea0d..a34a48827 100644 --- a/tests/dataconverter/test_helpers.py +++ b/tests/dataconverter/test_helpers.py @@ -25,6 +25,7 @@ import numpy as np import pytest + from pynxtools.dataconverter import helpers from pynxtools.dataconverter.template import Template from pynxtools.dataconverter.validation import validate_dict_against @@ -278,15 +279,14 @@ def fixture_filled_test_data(template, tmp_path): "not_a_num", ), ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/in" - "t_value should be one of: (, , , ," - " , , , , , " - ", , , , , ), as defined in " - "the NXDL as NX_INT." + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value " + "should be one of: (, , , , , , , , , , , , , , , ), as defined in " + "the NXDL as NX_INT" ), id="string-instead-of-int", ), From dd8beb4cbaf07c2c7894a2cb4c676148d890aebb Mon Sep 17 00:00:00 2001 From: Rubel Date: Wed, 19 Feb 2025 12:20:24 +0100 Subject: [PATCH 02/36] Still char instead of the int is being validated which is wrong. --- src/pynxtools/dataconverter/helpers.py | 70 +++++++++++++++++--------- tests/dataconverter/test_helpers.py | 46 ++++++++++++----- 2 files changed, 80 insertions(+), 36 deletions(-) diff --git a/src/pynxtools/dataconverter/helpers.py b/src/pynxtools/dataconverter/helpers.py index b9e99d1c6..602a8abc2 100644 --- a/src/pynxtools/dataconverter/helpers.py +++ b/src/pynxtools/dataconverter/helpers.py @@ -596,7 +596,9 @@ def is_value_valid_element_of_enum(value, elist) -> Tuple[bool, list]: np.signedinteger, ) np_float = (np.float16, np.float32, np.float64, np.floating) -np_bytes = (np.bytes_, np.byte, np.ubyte) +# Not to be confused with `np.byte` and `np.ubyte`, these store +# and integer of `8bit` and `unsigned 8bit` respectively. +np_bytes = (np.bytes_,) np_char = (np.str_, np.bytes_) # Only numpy Unicode string and Byte string np_bool = (np.bool_,) np_complex = (np.complex64, np.complex128, np.cdouble, np.csingle) @@ -604,59 +606,77 @@ def is_value_valid_element_of_enum(value, elist) -> Tuple[bool, list]: "ISO8601": (str,), "NX_BINARY": ( bytes, - bytearray, - np.ndarray, *np_bytes, ), - "NX_BOOLEAN": (bool, np.ndarray, *np_bool), + "NX_BOOLEAN": (bool, *np_bool), "NX_CHAR": (str, *np_char), "NX_DATE_TIME": (str,), - "NX_FLOAT": (float, np.ndarray, *np_float), - "NX_INT": (int, np.ndarray, *np_int), - "NX_UINT": (np.ndarray, np.unsignedinteger), + "NX_FLOAT": (float, *np_float), + "NX_INT": (int, *np_int), + "NX_UINT": ( + np.unsignedinteger, + np.uint, + ), "NX_NUMBER": ( int, float, - np.ndarray, *np_int, *np_float, - dict, ), "NX_POSINT": ( int, - np.ndarray, np.signedinteger, ), # > 0 is checked in is_valid_data_field() - "NX_COMPLEX": (complex, np.ndarray, *np_complex), - "NXDL_TYPE_UNAVAILABLE": (str,), # Defaults to a string if a type is not provided. + "NX_COMPLEX": (complex, *np_complex), + "NXDL_TYPE_UNAVAILABLE": ( + str, + *np_char, + ), # Defaults to a string if a type is not provided. "NX_CHAR_OR_NUMBER": ( str, int, float, - np.ndarray, *np_char, *np_int, *np_float, - dict, ), } -def check_all_children_for_callable(objects: list, check: Callable, *args) -> bool: - """Checks whether all objects in list are validated by given callable.""" - for obj in objects: - if not check(obj, *args): - return False +def check_all_children_for_callable( + objects: Union[list, np.ndarray], + checker: Optional[Callable] = None, + accepted_types: Optional[tuple] = None, +) -> bool: + """Checks whether all objects in list or numpy array are validated + by given callable and types. + """ - return True + if checker is not None: + for obj in objects: + args = (obj, accepted_types) if accepted_types is not None else (obj,) + if not checker(*args): + return False + return True + + # default checker + tmp_arr = None + if isinstance(objects, list): + # Handles list and list of list + tmp_arr = np.array(objects) + elif isinstance(objects, np.ndarray): + tmp_arr = objects + if tmp_arr is not None: + return any([np.issubdtype(tmp_arr.dtype, type_) for type_ in accepted_types]) def is_valid_data_type(value, accepted_types): """Checks whether the given value or its children are of an accepted type.""" - if not isinstance(value, list): + + if not isinstance(value, (list, np.ndarray)): return isinstance(value, accepted_types) - return check_all_children_for_callable(value, isinstance, accepted_types) + return check_all_children_for_callable(objects=value, accepted_types=accepted_types) def is_positive_int(value): @@ -666,7 +686,7 @@ def is_greater_than(num): return num.flat[0] > 0 if isinstance(num, np.ndarray) else num > 0 if isinstance(value, list): - return check_all_children_for_callable(value, is_greater_than) + return check_all_children_for_callable(objects=value, checker=is_greater_than) return value.flat[0] > 0 if isinstance(value, np.ndarray) else value > 0 @@ -707,7 +727,9 @@ def is_valid_data_field(value, nxdl_type, path): if value is None: raise ValueError output_value = accepted_types[0](value) - except ValueError: + ## TODO recheck the the TypeError after discussion with Lev + ## of the above line + except (ValueError, TypeError): collector.collect_and_log( path, ValidationProblem.InvalidType, accepted_types, nxdl_type ) diff --git a/tests/dataconverter/test_helpers.py b/tests/dataconverter/test_helpers.py index a34a48827..1352ce6b5 100644 --- a/tests/dataconverter/test_helpers.py +++ b/tests/dataconverter/test_helpers.py @@ -279,14 +279,13 @@ def fixture_filled_test_data(template, tmp_path): "not_a_num", ), ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value " - "should be one of: (, , , , , , , , , , , , , , , ), as defined in " - "the NXDL as NX_INT" + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value should " + "be one of: (, , " + ", , , " + ", , , " + ", , , " + ", , , ), as defined in the NXDL as NX_INT" ), id="string-instead-of-int", ), @@ -297,9 +296,9 @@ def fixture_filled_test_data(template, tmp_path): "NOT_TRUE_OR_FALSE", ), ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value sh" - "ould be one of: (, , , ), as defined in the " + "NXDL as NX_BOOLEAN" ), id="string-instead-of-int", ), @@ -327,12 +326,22 @@ def fixture_filled_test_data(template, tmp_path): TEMPLATE, "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", 3 ), ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value should be of Python type:" + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value should " + "be of Python type:" " (, , )," " as defined in the NXDL as NX_CHAR." ), id="int-instead-of-chars", ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", + ["list", "of", "chars"], + ), + "", + id="list-of-char-instead-of-chars", + ), pytest.param( alter_dict( TEMPLATE, "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value", None @@ -340,6 +349,18 @@ def fixture_filled_test_data(template, tmp_path): "", id="empty-optional-field", ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value", + [2], # pylint: disable=E1126 + ), + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value should be" + " one of: (, , " + ", , ), as defined in the " + "NXDL as NX_FLOAT", + id="list-of-int-instead-of-float", + ), pytest.param( set_to_none_in_dict( TEMPLATE, @@ -534,6 +555,7 @@ def test_validate_data_dict(caplog, data_dict, error_message, request): "link-dict-instead-of-bool", "opt-group-completely-removed", "required-field-provided-in-variadic-optional-group", + "list-of-char-instead-of-chars", ): with caplog.at_level(logging.WARNING): assert validate_dict_against("NXtest", data_dict)[0] From 351f377db1c16e70866f36b9758fca220f1f371a Mon Sep 17 00:00:00 2001 From: Rubel Date: Thu, 20 Feb 2025 10:08:08 +0100 Subject: [PATCH 03/36] Remove auto conversion for datatype. --- src/pynxtools/dataconverter/helpers.py | 30 ++++++++++++----------- src/pynxtools/dataconverter/validation.py | 4 +-- tests/dataconverter/test_helpers.py | 29 +++++++++++----------- 3 files changed, 32 insertions(+), 31 deletions(-) diff --git a/src/pynxtools/dataconverter/helpers.py b/src/pynxtools/dataconverter/helpers.py index 602a8abc2..39429ada6 100644 --- a/src/pynxtools/dataconverter/helpers.py +++ b/src/pynxtools/dataconverter/helpers.py @@ -668,6 +668,7 @@ def check_all_children_for_callable( tmp_arr = objects if tmp_arr is not None: return any([np.issubdtype(tmp_arr.dtype, type_) for type_ in accepted_types]) + return False def is_valid_data_type(value, accepted_types): @@ -706,29 +707,30 @@ def convert_str_to_bool_safe(value): def is_valid_data_field(value, nxdl_type, path): # todo: Check this funciton and wtire test for it. It seems the funciton is not # working as expected. - """Checks whether a given value is valid according to what is defined in the NXDL. - - This function will also try to convert typical types, for example int to float, - and return the successful conversion. + """Checks whether a given value is valid according to the type defined in the NXDL. - If it fails to convert, it raises an Exception. + This function also converts bool value comes in str format. In case, it fails to + convert, it raises an Exception. - Returns two values: first, boolean (True if the the value corresponds to nxdl_type, - False otherwise) and second, result of attempted conversion or the original value - (if conversion is not needed or impossible) + Returns two values: + boolean (True if the the value corresponds to nxdl_type, False otherwise) + converted_value bool value. """ - accepted_types = NEXUS_TO_PYTHON_DATA_TYPES[nxdl_type] - output_value = value + accepted_types = NEXUS_TO_PYTHON_DATA_TYPES[nxdl_type] + # Do not count the dict as it represents a link value if not isinstance(value, dict) and not is_valid_data_type(value, accepted_types): try: if accepted_types[0] is bool and isinstance(value, str): value = convert_str_to_bool_safe(value) if value is None: raise ValueError - output_value = accepted_types[0](value) - ## TODO recheck the the TypeError after discussion with Lev - ## of the above line + return True, value + + collector.collect_and_log( + path, ValidationProblem.InvalidType, accepted_types, nxdl_type + ) + return False, value except (ValueError, TypeError): collector.collect_and_log( path, ValidationProblem.InvalidType, accepted_types, nxdl_type @@ -749,7 +751,7 @@ def is_valid_data_field(value, nxdl_type, path): collector.collect_and_log(path, ValidationProblem.InvalidDatetime, value) return False, value - return True, output_value + return True, value @lru_cache(maxsize=None) diff --git a/src/pynxtools/dataconverter/validation.py b/src/pynxtools/dataconverter/validation.py index 4b599b43a..cf2a4c0f1 100644 --- a/src/pynxtools/dataconverter/validation.py +++ b/src/pynxtools/dataconverter/validation.py @@ -422,7 +422,7 @@ def handle_field(node: NexusNode, keys: Mapping[str, Any], prev_path: str): continue # Check general validity - is_valid_data_field( + _, _ = is_valid_data_field( mapping[f"{prev_path}/{variant}"], node.dtype, f"{prev_path}/{variant}" ) @@ -468,7 +468,7 @@ def handle_attribute(node: NexusNode, keys: Mapping[str, Any], prev_path: str): return for variant in variants: - is_valid_data_field( + _, _ = is_valid_data_field( mapping[ f"{prev_path}/{variant if variant.startswith('@') else f'@{variant}'}" ], diff --git a/tests/dataconverter/test_helpers.py b/tests/dataconverter/test_helpers.py index 1352ce6b5..12ce9f0f1 100644 --- a/tests/dataconverter/test_helpers.py +++ b/tests/dataconverter/test_helpers.py @@ -279,13 +279,13 @@ def fixture_filled_test_data(template, tmp_path): "not_a_num", ), ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value should " - "be one of: (, , " - ", , , " - ", , , " - ", , , " - ", , , ), as defined in the NXDL as NX_INT" + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value should be" + " one of: (, , ," + " , , , <" + "class 'numpy.int32'>, , , <" + "class 'numpy.uint16'>, , , <" + "class 'numpy.uint64'>, , ), as defined in the NXDL as NX_INT." ), id="string-instead-of-int", ), @@ -300,7 +300,7 @@ def fixture_filled_test_data(template, tmp_path): "be one of: (, ), as defined in the " "NXDL as NX_BOOLEAN" ), - id="string-instead-of-int", + id="string-instead-of-bool", ), pytest.param( alter_dict( @@ -309,7 +309,7 @@ def fixture_filled_test_data(template, tmp_path): {"link": "/a-link"}, ), (""), - id="link-dict-instead-of-bool", + id="link-dict-instead-of-int", ), pytest.param( alter_dict( @@ -326,13 +326,13 @@ def fixture_filled_test_data(template, tmp_path): TEMPLATE, "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", 3 ), ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value should " - "be of Python type:" - " (, , )," - " as defined in the NXDL as NX_CHAR." + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value should" + " be one of: (, , " + "), as defined in the NXDL as NX_CHAR." ), id="int-instead-of-chars", ), + # TODO add test array of char pytest.param( alter_dict( TEMPLATE, @@ -551,8 +551,7 @@ def test_validate_data_dict(caplog, data_dict, error_message, request): "UTC-with-+00:00", "UTC-with-Z", "no-child-provided-optional-parent", - "int-instead-of-chars", - "link-dict-instead-of-bool", + "link-dict-instead-of-int", "opt-group-completely-removed", "required-field-provided-in-variadic-optional-group", "list-of-char-instead-of-chars", From 00b78aaf90d7ad3c99a38ea9966e0270fe6ca1e4 Mon Sep 17 00:00:00 2001 From: Rubel Date: Thu, 20 Feb 2025 11:24:46 +0100 Subject: [PATCH 04/36] extends tests. --- tests/dataconverter/test_helpers.py | 154 +++++++++++++++++++++++++++- 1 file changed, 152 insertions(+), 2 deletions(-) diff --git a/tests/dataconverter/test_helpers.py b/tests/dataconverter/test_helpers.py index 12ce9f0f1..008b14b02 100644 --- a/tests/dataconverter/test_helpers.py +++ b/tests/dataconverter/test_helpers.py @@ -289,6 +289,58 @@ def fixture_filled_test_data(template, tmp_path): ), id="string-instead-of-int", ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value", + ["1", "2", "3"], + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value should be" + " one of: (, , ," + " , , , <" + "class 'numpy.int32'>, , , <" + "class 'numpy.uint16'>, , , <" + "class 'numpy.uint64'>, , ), as defined in the NXDL as NX_INT." + ), + id="list-of-int-str-instead-of-int", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value", + np.array([2.0, 3.0, 4.0], dtype=np.float32), + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value should be" + " one of: (, , ," + " , , , <" + "class 'numpy.int32'>, , , <" + "class 'numpy.uint16'>, , , <" + "class 'numpy.uint64'>, , ), as defined in the NXDL as NX_INT." + ), + id="array-of-float-instead-of-int", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value", + [2, 3, 4], + ), + (""), + id="List-of-int-instead-of-int", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value", + np.array([2, 3, 4], dtype=np.int32), + ), + (""), + id="array-of-int32-instead-of-int", + ), pytest.param( alter_dict( TEMPLATE, @@ -321,6 +373,48 @@ def fixture_filled_test_data(template, tmp_path): ), id="negative-posint", ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value", + [-1, 2], + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value " + "should be a positive int, but is [-1, 2]." + ), + id="negative-posint-list", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value", + np.array([-1, 2], dtype=np.int8), + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value should" + " be a positive int, but is [-1 2]." + ), + id="negative-posint-array", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value", + [1, 2], + ), + (""), + id="positive-posint-list", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value", + np.array([1, 2], dtype=np.int8), + ), + (""), + id="positive-posint-array", + ), pytest.param( alter_dict( TEMPLATE, "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", 3 @@ -332,7 +426,35 @@ def fixture_filled_test_data(template, tmp_path): ), id="int-instead-of-chars", ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", + np.array(["1", "2", "3"], dtype=np.str_), + ), + (""), + id="array-of-chars", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", + np.array(["1", "2", "3"], dtype=np.bytes_), + ), + (""), + id="array-of-bytes-chars", + ), + # pytest.param( + # alter_dict( + # TEMPLATE, + # "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", + # np.char.chararray(["1", "2", "3"]), + # ), + # (""), + # id="numpy-chararray", + # ), # TODO add test array of char + # TODO add test for numpy array of char and chararray pytest.param( alter_dict( TEMPLATE, @@ -340,7 +462,7 @@ def fixture_filled_test_data(template, tmp_path): ["list", "of", "chars"], ), "", - id="list-of-char-instead-of-chars", + id="list-of-string-instead-of-chars", ), pytest.param( alter_dict( @@ -349,6 +471,27 @@ def fixture_filled_test_data(template, tmp_path): "", id="empty-optional-field", ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value", + np.array([2.0, 3.0, 4.0], dtype=np.float32), + ), + "", + id="array-of-float-instead-of-float", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value", + np.array(["2.0", "3.0"], dtype=np.str_), + ), + " The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value should " + "be one of: (, , , , ), as defined in the " + "NXDL as NX_FLOAT.", + id="array-of-str-instead-of-float", + ), pytest.param( alter_dict( TEMPLATE, @@ -554,7 +697,14 @@ def test_validate_data_dict(caplog, data_dict, error_message, request): "link-dict-instead-of-int", "opt-group-completely-removed", "required-field-provided-in-variadic-optional-group", - "list-of-char-instead-of-chars", + "list-of-string-instead-of-chars", + "array-of-int32-instead-of-int", + "List-of-int-instead-of-int", + "positive-posint-list", + "positive-posint-array", + "array-of-chars", + "array-of-bytes-chars", + "array-of-float-instead-of-float", ): with caplog.at_level(logging.WARNING): assert validate_dict_against("NXtest", data_dict)[0] From 3ad40a0f0b4028e40cc6f50882717f3cc5920b37 Mon Sep 17 00:00:00 2001 From: Rubel Date: Thu, 27 Feb 2025 16:04:06 +0100 Subject: [PATCH 05/36] resolve PR comments --- src/pynxtools/dataconverter/helpers.py | 24 +++++++++++------------ src/pynxtools/dataconverter/validation.py | 4 ++-- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/pynxtools/dataconverter/helpers.py b/src/pynxtools/dataconverter/helpers.py index 39429ada6..3703faa33 100644 --- a/src/pynxtools/dataconverter/helpers.py +++ b/src/pynxtools/dataconverter/helpers.py @@ -597,7 +597,7 @@ def is_value_valid_element_of_enum(value, elist) -> Tuple[bool, list]: ) np_float = (np.float16, np.float32, np.float64, np.floating) # Not to be confused with `np.byte` and `np.ubyte`, these store -# and integer of `8bit` and `unsigned 8bit` respectively. +# an integer of `8bit` and `unsigned 8bit` respectively. np_bytes = (np.bytes_,) np_char = (np.str_, np.bytes_) # Only numpy Unicode string and Byte string np_bool = (np.bool_,) @@ -658,16 +658,14 @@ def check_all_children_for_callable( if not checker(*args): return False return True - - # default checker - tmp_arr = None + if isinstance(objects, tuple): + return False if isinstance(objects, list): # Handles list and list of list - tmp_arr = np.array(objects) - elif isinstance(objects, np.ndarray): - tmp_arr = objects - if tmp_arr is not None: - return any([np.issubdtype(tmp_arr.dtype, type_) for type_ in accepted_types]) + return all([type(elem) in accepted_types for elem in objects]) + if isinstance(objects, np.ndarray): + return any([np.issubdtype(objects.dtype, type_) for type_ in accepted_types]) + return False @@ -709,12 +707,12 @@ def is_valid_data_field(value, nxdl_type, path): # working as expected. """Checks whether a given value is valid according to the type defined in the NXDL. - This function also converts bool value comes in str format. In case, it fails to - convert, it raises an Exception. + This function only tries to convert boolean value in str format (e.g. "true" ) to + python Boolean (True). In case, it fails to convert, it raises an Exception. Returns two values: - boolean (True if the the value corresponds to nxdl_type, False otherwise) - converted_value bool value. + Bool: (True if the the value corresponds to nxdl_type, False otherwise) + Any: Converted_value bool value if possible otherwise original value. """ accepted_types = NEXUS_TO_PYTHON_DATA_TYPES[nxdl_type] diff --git a/src/pynxtools/dataconverter/validation.py b/src/pynxtools/dataconverter/validation.py index cf2a4c0f1..b2c14c355 100644 --- a/src/pynxtools/dataconverter/validation.py +++ b/src/pynxtools/dataconverter/validation.py @@ -534,8 +534,8 @@ def is_documented(key: str, node: NexusNode) -> bool: collector.collect_and_log( f"{key}", ValidationProblem.MissingUnit, node.unit ) - - return is_valid_data_field(mapping[key], node.dtype, key)[0] + is_documented_flag, _ = is_valid_data_field(mapping[key], node.dtype, key) + return is_documented_flag def recurse_tree( node: NexusNode, From 6cf6f6d50153beb956b69c6e919fe72aecfe9fd1 Mon Sep 17 00:00:00 2001 From: Rubel Date: Thu, 27 Feb 2025 16:43:41 +0100 Subject: [PATCH 06/36] Remove unnecessary returned value. --- src/pynxtools/dataconverter/helpers.py | 12 ++++++------ src/pynxtools/dataconverter/validation.py | 6 +++--- tests/dataconverter/test_helpers.py | 4 +++- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/pynxtools/dataconverter/helpers.py b/src/pynxtools/dataconverter/helpers.py index 3703faa33..4274dc888 100644 --- a/src/pynxtools/dataconverter/helpers.py +++ b/src/pynxtools/dataconverter/helpers.py @@ -723,21 +723,21 @@ def is_valid_data_field(value, nxdl_type, path): value = convert_str_to_bool_safe(value) if value is None: raise ValueError - return True, value + return True collector.collect_and_log( path, ValidationProblem.InvalidType, accepted_types, nxdl_type ) - return False, value + return False except (ValueError, TypeError): collector.collect_and_log( path, ValidationProblem.InvalidType, accepted_types, nxdl_type ) - return False, value + return False if nxdl_type == "NX_POSINT" and not is_positive_int(value): collector.collect_and_log(path, ValidationProblem.IsNotPosInt, value) - return False, value + return False if nxdl_type in ("ISO8601", "NX_DATE_TIME"): iso8601 = re.compile( @@ -747,9 +747,9 @@ def is_valid_data_field(value, nxdl_type, path): results = iso8601.search(value) if results is None: collector.collect_and_log(path, ValidationProblem.InvalidDatetime, value) - return False, value + return False - return True, value + return True @lru_cache(maxsize=None) diff --git a/src/pynxtools/dataconverter/validation.py b/src/pynxtools/dataconverter/validation.py index b2c14c355..23d4f4734 100644 --- a/src/pynxtools/dataconverter/validation.py +++ b/src/pynxtools/dataconverter/validation.py @@ -422,7 +422,7 @@ def handle_field(node: NexusNode, keys: Mapping[str, Any], prev_path: str): continue # Check general validity - _, _ = is_valid_data_field( + _ = is_valid_data_field( mapping[f"{prev_path}/{variant}"], node.dtype, f"{prev_path}/{variant}" ) @@ -468,7 +468,7 @@ def handle_attribute(node: NexusNode, keys: Mapping[str, Any], prev_path: str): return for variant in variants: - _, _ = is_valid_data_field( + _ = is_valid_data_field( mapping[ f"{prev_path}/{variant if variant.startswith('@') else f'@{variant}'}" ], @@ -534,7 +534,7 @@ def is_documented(key: str, node: NexusNode) -> bool: collector.collect_and_log( f"{key}", ValidationProblem.MissingUnit, node.unit ) - is_documented_flag, _ = is_valid_data_field(mapping[key], node.dtype, key) + is_documented_flag = is_valid_data_field(mapping[key], node.dtype, key) return is_documented_flag def recurse_tree( diff --git a/tests/dataconverter/test_helpers.py b/tests/dataconverter/test_helpers.py index 008b14b02..119426c40 100644 --- a/tests/dataconverter/test_helpers.py +++ b/tests/dataconverter/test_helpers.py @@ -97,7 +97,9 @@ def listify_template(data_dict: Template): "type", "definition", "date_value", - ): + ) or isinstance( + data_dict[optionality][path], np.ndarray + ): # avoid list numpy array listified_template[optionality][path] = data_dict[optionality][path] else: listified_template[optionality][path] = [data_dict[optionality][path]] From 539bdeaff8847ae25be539ce10589eda355000d5 Mon Sep 17 00:00:00 2001 From: Rubel Date: Thu, 27 Feb 2025 17:49:23 +0100 Subject: [PATCH 07/36] fix np integer and float. --- src/pynxtools/dataconverter/helpers.py | 24 +++------------- tests/dataconverter/test_helpers.py | 38 ++++++++------------------ 2 files changed, 15 insertions(+), 47 deletions(-) diff --git a/src/pynxtools/dataconverter/helpers.py b/src/pynxtools/dataconverter/helpers.py index 4274dc888..54751a0c1 100644 --- a/src/pynxtools/dataconverter/helpers.py +++ b/src/pynxtools/dataconverter/helpers.py @@ -579,29 +579,14 @@ def is_value_valid_element_of_enum(value, elist) -> Tuple[bool, list]: NUMPY_INT_TYPES = (np.short, np.intc, np.int_) NUMPY_UINT_TYPES = (np.ushort, np.uintc, np.uint) # np int for np version 1.26.0 -np_int = ( - np.intc, - np.int_, - np.intp, - np.int8, - np.int16, - np.int32, - np.int64, - np.uint8, - np.uint16, - np.uint32, - np.uint64, - np.uint, - np.unsignedinteger, - np.signedinteger, -) -np_float = (np.float16, np.float32, np.float64, np.floating) +np_int = (np.integer,) +np_float = (np.floating,) # Not to be confused with `np.byte` and `np.ubyte`, these store # an integer of `8bit` and `unsigned 8bit` respectively. np_bytes = (np.bytes_,) np_char = (np.str_, np.bytes_) # Only numpy Unicode string and Byte string np_bool = (np.bool_,) -np_complex = (np.complex64, np.complex128, np.cdouble, np.csingle) +np_complex = (np.complex64, np.complex128, np.cdouble, np.csingle, np.complex_) NEXUS_TO_PYTHON_DATA_TYPES = { "ISO8601": (str,), "NX_BINARY": ( @@ -710,9 +695,8 @@ def is_valid_data_field(value, nxdl_type, path): This function only tries to convert boolean value in str format (e.g. "true" ) to python Boolean (True). In case, it fails to convert, it raises an Exception. - Returns two values: + Return: Bool: (True if the the value corresponds to nxdl_type, False otherwise) - Any: Converted_value bool value if possible otherwise original value. """ accepted_types = NEXUS_TO_PYTHON_DATA_TYPES[nxdl_type] diff --git a/tests/dataconverter/test_helpers.py b/tests/dataconverter/test_helpers.py index 119426c40..8059e00e2 100644 --- a/tests/dataconverter/test_helpers.py +++ b/tests/dataconverter/test_helpers.py @@ -281,13 +281,9 @@ def fixture_filled_test_data(template, tmp_path): "not_a_num", ), ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value should be" - " one of: (, , ," - " , , , <" - "class 'numpy.int32'>, , , <" - "class 'numpy.uint16'>, , , <" - "class 'numpy.uint64'>, , ), as defined in the NXDL as NX_INT." + " The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value should" + " be one of: (, ), as defined in the" + " NXDL as NX_INT.\n" ), id="string-instead-of-int", ), @@ -298,13 +294,8 @@ def fixture_filled_test_data(template, tmp_path): ["1", "2", "3"], ), ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value should be" - " one of: (, , ," - " , , , <" - "class 'numpy.int32'>, , , <" - "class 'numpy.uint16'>, , , <" - "class 'numpy.uint64'>, , ), as defined in the NXDL as NX_INT." + " The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value should" + " be one of: (, )" ), id="list-of-int-str-instead-of-int", ), @@ -316,12 +307,7 @@ def fixture_filled_test_data(template, tmp_path): ), ( "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value should be" - " one of: (, , ," - " , , , <" - "class 'numpy.int32'>, , , <" - "class 'numpy.uint16'>, , , <" - "class 'numpy.uint64'>, , ), as defined in the NXDL as NX_INT." + " one of: (, )" ), id="array-of-float-instead-of-int", ), @@ -488,10 +474,9 @@ def fixture_filled_test_data(template, tmp_path): "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value", np.array(["2.0", "3.0"], dtype=np.str_), ), - " The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value should " - "be one of: (, , , , ), as defined in the " - "NXDL as NX_FLOAT.", + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value should be " + "one of: (, ), as defined in the NXDL " + "as NX_FLOAT.\n", id="array-of-str-instead-of-float", ), pytest.param( @@ -501,9 +486,8 @@ def fixture_filled_test_data(template, tmp_path): [2], # pylint: disable=E1126 ), "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value should be" - " one of: (, , " - ", , ), as defined in the " - "NXDL as NX_FLOAT", + " one of: (, ), as defined in the NXDL" + " as NX_FLOAT.\n", id="list-of-int-instead-of-float", ), pytest.param( From cde7a6e12ab95f7bbcdc4359cc9f1ba3709f0e82 Mon Sep 17 00:00:00 2001 From: Rubel Date: Fri, 28 Feb 2025 11:20:27 +0100 Subject: [PATCH 08/36] minor change. --- src/pynxtools/dataconverter/validation.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/pynxtools/dataconverter/validation.py b/src/pynxtools/dataconverter/validation.py index 23d4f4734..a8ec77f2d 100644 --- a/src/pynxtools/dataconverter/validation.py +++ b/src/pynxtools/dataconverter/validation.py @@ -534,8 +534,7 @@ def is_documented(key: str, node: NexusNode) -> bool: collector.collect_and_log( f"{key}", ValidationProblem.MissingUnit, node.unit ) - is_documented_flag = is_valid_data_field(mapping[key], node.dtype, key) - return is_documented_flag + return is_valid_data_field(mapping[key], node.dtype, key) def recurse_tree( node: NexusNode, From 4ab4b53545505cb561a7b253fd0f26026414bb18 Mon Sep 17 00:00:00 2001 From: rettigl Date: Fri, 28 Feb 2025 12:21:23 +0100 Subject: [PATCH 09/36] check enums for documented fields, and don't return False if any issue with documented keys --- src/pynxtools/dataconverter/validation.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/pynxtools/dataconverter/validation.py b/src/pynxtools/dataconverter/validation.py index a8ec77f2d..ca467c756 100644 --- a/src/pynxtools/dataconverter/validation.py +++ b/src/pynxtools/dataconverter/validation.py @@ -526,6 +526,21 @@ def is_documented(key: str, node: NexusNode) -> bool: if "@" in key and node.type != "attribute": return False + # if we arrive here, the key is supposed to be documented. + # We still do some further checks before returning. + + # Check general validity + _ = is_valid_data_field(mapping[key], node.dtype, key) + + # Check enumeration + if node.items is not None and mapping[key] not in node.items: + collector.collect_and_log( + key, + ValidationProblem.InvalidEnum, + node.items, + ) + + # Check main field exists for units if ( isinstance(node, NexusEntity) and node.unit is not None @@ -534,7 +549,8 @@ def is_documented(key: str, node: NexusNode) -> bool: collector.collect_and_log( f"{key}", ValidationProblem.MissingUnit, node.unit ) - return is_valid_data_field(mapping[key], node.dtype, key) + + return True def recurse_tree( node: NexusNode, From ae109162cefcfaa810c394e06ea2d72a082c7817 Mon Sep 17 00:00:00 2001 From: rettigl Date: Tue, 18 Feb 2025 17:17:18 +0100 Subject: [PATCH 10/36] add enum checking for attributes --- src/pynxtools/dataconverter/validation.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/pynxtools/dataconverter/validation.py b/src/pynxtools/dataconverter/validation.py index ca467c756..80a8bc32c 100644 --- a/src/pynxtools/dataconverter/validation.py +++ b/src/pynxtools/dataconverter/validation.py @@ -476,6 +476,20 @@ def handle_attribute(node: NexusNode, keys: Mapping[str, Any], prev_path: str): f"{prev_path}/{variant if variant.startswith('@') else f'@{variant}'}", ) + # Check enumeration + if ( + node.items is not None + and mapping[ + f"{prev_path}/{variant if variant.startswith('@') else f'@{variant}'}" + ] + not in node.items + ): + collector.collect_and_log( + f"{prev_path}/{variant if variant.startswith('@') else f'@{variant}'}", + ValidationProblem.InvalidEnum, + node.items, + ) + def handle_choice(node: NexusNode, keys: Mapping[str, Any], prev_path: str): global collector old_collector = collector From 8f4291e9bdb0f123b438d99c690111797e96b521 Mon Sep 17 00:00:00 2001 From: Sherjeel Shabih Date: Mon, 24 Feb 2025 15:16:49 +0100 Subject: [PATCH 11/36] Adds parsing code for enumeration tree generation during validation --- src/pynxtools/dataconverter/helpers.py | 2 +- src/pynxtools/dataconverter/nexus_tree.py | 17 +++++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/pynxtools/dataconverter/helpers.py b/src/pynxtools/dataconverter/helpers.py index 54751a0c1..9e95779ed 100644 --- a/src/pynxtools/dataconverter/helpers.py +++ b/src/pynxtools/dataconverter/helpers.py @@ -85,7 +85,7 @@ 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 on of the following strings: {value}" + f"The value at {path} should be one of the following: {value}" ) elif log_type == ValidationProblem.MissingRequiredGroup: logger.warning(f"The required group, {path}, hasn't been supplied.") diff --git a/src/pynxtools/dataconverter/nexus_tree.py b/src/pynxtools/dataconverter/nexus_tree.py index bbba22c09..77349df49 100644 --- a/src/pynxtools/dataconverter/nexus_tree.py +++ b/src/pynxtools/dataconverter/nexus_tree.py @@ -761,7 +761,7 @@ class NexusEntity(NexusNode): type: Literal["field", "attribute"] unit: Optional[NexusUnitCategory] = None dtype: NexusType = "NX_CHAR" - items: Optional[List[str]] = None + items: Optional[List[Any]] = None shape: Optional[Tuple[Optional[int], ...]] = None def _set_type(self): @@ -790,14 +790,23 @@ def _set_items(self): based on the values in the inheritance chain. The first vale found is used. """ - if not self.dtype == "NX_CHAR": - return for elem in self.inheritance: enum = elem.find(f"nx:enumeration", namespaces=namespaces) if enum is not None: self.items = [] for items in enum.findall(f"nx:item", namespaces=namespaces): - self.items.append(items.attrib["value"]) + value = items.attrib["value"] + if value[0] == "[" and value[-1] == "]": + import ast + + try: + self.items.append(ast.literal_eval(value)) + except (ValueError, SyntaxError): + raise Exception( + f"Error parsing enumeration item in the provided NXDL: {value}" + ) + else: + self.items.append(value) return def _set_shape(self): From 099b965245253509c18c826449203dcee373565c Mon Sep 17 00:00:00 2001 From: Sherjeel Shabih Date: Mon, 24 Feb 2025 15:23:22 +0100 Subject: [PATCH 12/36] Fix typos in old test --- tests/dataconverter/test_helpers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/dataconverter/test_helpers.py b/tests/dataconverter/test_helpers.py index 8059e00e2..afa8ff71e 100644 --- a/tests/dataconverter/test_helpers.py +++ b/tests/dataconverter/test_helpers.py @@ -583,8 +583,8 @@ def fixture_filled_test_data(template, tmp_path): ), ( "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/type should " - "be on of the following" - " strings: ['1st type', '2nd type', '3rd type', '4th type']" + "be one of the following" + ": ['1st type', '2nd type', '3rd type', '4th type']" ), id="wrong-enum-choice", ), From 4da57353bfec211bce6554aee293fcd9eb339deb Mon Sep 17 00:00:00 2001 From: rettigl Date: Tue, 25 Feb 2025 23:51:11 +0100 Subject: [PATCH 13/36] always check data types and enums, and check NXdata attributes separately --- src/pynxtools/dataconverter/validation.py | 31 +++++++++++++++-------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/src/pynxtools/dataconverter/validation.py b/src/pynxtools/dataconverter/validation.py index 80a8bc32c..db5b02579 100644 --- a/src/pynxtools/dataconverter/validation.py +++ b/src/pynxtools/dataconverter/validation.py @@ -248,6 +248,14 @@ def check_nxdata(): prev_path=prev_path, ) + # check NXdata attributes + for attr in ("signal", "auxiliary_signals", "axes"): + handle_attribute( + node.search_add_child_for(attr), + keys, + prev_path=prev_path, + ) + for i, axis in enumerate(axes): if axis == ".": continue @@ -392,12 +400,12 @@ def _follow_link( def handle_field(node: NexusNode, keys: Mapping[str, Any], prev_path: str): full_path = remove_from_not_visited(f"{prev_path}/{node.name}") variants = get_variations_of(node, keys) - if not variants: - if node.optionality == "required" and node.type in missing_type_err: - collector.collect_and_log( - full_path, missing_type_err.get(node.type), None - ) - + if ( + not variants + and node.optionality == "required" + and node.type in missing_type_err + ): + collector.collect_and_log(full_path, missing_type_err.get(node.type), None) return for variant in variants: @@ -460,11 +468,12 @@ def handle_field(node: NexusNode, keys: Mapping[str, Any], prev_path: str): def handle_attribute(node: NexusNode, keys: Mapping[str, Any], prev_path: str): full_path = remove_from_not_visited(f"{prev_path}/@{node.name}") variants = get_variations_of(node, keys) - if not variants: - if node.optionality == "required" and node.type in missing_type_err: - collector.collect_and_log( - full_path, missing_type_err.get(node.type), None - ) + if ( + not variants + and node.optionality == "required" + and node.type in missing_type_err + ): + collector.collect_and_log(full_path, missing_type_err.get(node.type), None) return for variant in variants: From d4f4c2fc3b6a7d8ca791f88e35f6f1067b0b2ba9 Mon Sep 17 00:00:00 2001 From: rettigl Date: Wed, 26 Feb 2025 14:15:03 +0100 Subject: [PATCH 14/36] fix typos --- src/pynxtools/dataconverter/validation.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pynxtools/dataconverter/validation.py b/src/pynxtools/dataconverter/validation.py index db5b02579..aade2d9b2 100644 --- a/src/pynxtools/dataconverter/validation.py +++ b/src/pynxtools/dataconverter/validation.py @@ -168,7 +168,7 @@ def validate_dict_against( appdef: str, mapping: Mapping[str, Any], ignore_undocumented: bool = False ) -> Tuple[bool, List]: """ - Validates a mapping against the NeXus tree for applicationd definition `appdef`. + Validates a mapping against the NeXus tree for application definition `appdef`. Args: appdef (str): The appdef name to validate against. @@ -410,7 +410,7 @@ def handle_field(node: NexusNode, keys: Mapping[str, Any], prev_path: str): for variant in variants: if node.optionality == "required" and isinstance(keys[variant], Mapping): - # Check if all fields in the dict are actual attributes (startwith @) + # Check if all fields in the dict are actual attributes (startswith @) all_attrs = True for entry in keys[variant]: if not entry.startswith("@"): @@ -594,7 +594,7 @@ def check_attributes_of_nonexisting_field( ) -> list: """ This method runs through the mapping dictionary and checks if there are any - attributes assigned to the fields (not groups!) which are not expicitly + attributes assigned to the fields (not groups!) which are not explicitly present in the mapping. If there are any found, a warning is logged and the corresponding items are added to the list returned by the method. @@ -695,7 +695,7 @@ def check_type_with_tree( if (next_child_class is not None) or (next_child_name is not None): output = None for child in node.children: - # regexs to separarte the class and the name from full name of the child + # regexs to separate the class and the name from full name of the child child_class_from_node = re.sub( r"(\@.*)*(\[.*?\])*(\(.*?\))*([a-z]\_)*(\_[a-z])*[a-z]*\s*", "", From 1925d5df0a9d42f25b79fdbc63c740405865248d Mon Sep 17 00:00:00 2001 From: rettigl Date: Fri, 28 Feb 2025 13:11:43 +0100 Subject: [PATCH 15/36] move enum checking into is_valid_data_field, and proper bool conversion --- src/pynxtools/dataconverter/helpers.py | 44 +++++++++++---------- src/pynxtools/dataconverter/validation.py | 47 +++++------------------ 2 files changed, 33 insertions(+), 58 deletions(-) diff --git a/src/pynxtools/dataconverter/helpers.py b/src/pynxtools/dataconverter/helpers.py index 9e95779ed..307bbbd3d 100644 --- a/src/pynxtools/dataconverter/helpers.py +++ b/src/pynxtools/dataconverter/helpers.py @@ -675,20 +675,20 @@ def is_greater_than(num): return value.flat[0] > 0 if isinstance(value, np.ndarray) else value > 0 -def convert_str_to_bool_safe(value): +def convert_str_to_bool_safe(value: str) -> Optional[bool]: """Only returns True or False if someone mistakenly adds quotation marks but mean a bool. - For everything else it returns None. + For everything else it raises a ValueError. """ if value.lower() == "true": return True if value.lower() == "false": return False - return None + raise ValueError(f"Could not interpret string '{value}' as boolean.") -def is_valid_data_field(value, nxdl_type, path): - # todo: Check this funciton and wtire test for it. It seems the funciton is not +def is_valid_data_field(value: Any, nxdl_type: str, nxdl_enum: list, path: str) -> Any: + # todo: Check this function and write test for it. It seems the function is not # working as expected. """Checks whether a given value is valid according to the type defined in the NXDL. @@ -696,32 +696,27 @@ def is_valid_data_field(value, nxdl_type, path): python Boolean (True). In case, it fails to convert, it raises an Exception. Return: - Bool: (True if the the value corresponds to nxdl_type, False otherwise) + value: the possibly converted data value """ accepted_types = NEXUS_TO_PYTHON_DATA_TYPES[nxdl_type] # Do not count the dict as it represents a link value if not isinstance(value, dict) and not is_valid_data_type(value, accepted_types): - try: - if accepted_types[0] is bool and isinstance(value, str): + # try to convert string to bool + if accepted_types[0] is bool and isinstance(value, str): + try: value = convert_str_to_bool_safe(value) - if value is None: - raise ValueError - return True - - collector.collect_and_log( - path, ValidationProblem.InvalidType, accepted_types, nxdl_type - ) - return False - except (ValueError, TypeError): + except (ValueError, TypeError): + collector.collect_and_log( + path, ValidationProblem.InvalidType, accepted_types, nxdl_type + ) + else: collector.collect_and_log( path, ValidationProblem.InvalidType, accepted_types, nxdl_type ) - return False if nxdl_type == "NX_POSINT" and not is_positive_int(value): collector.collect_and_log(path, ValidationProblem.IsNotPosInt, value) - return False if nxdl_type in ("ISO8601", "NX_DATE_TIME"): iso8601 = re.compile( @@ -731,9 +726,16 @@ def is_valid_data_field(value, nxdl_type, path): results = iso8601.search(value) if results is None: collector.collect_and_log(path, ValidationProblem.InvalidDatetime, value) - return False - return True + # Check enumeration + if nxdl_enum is not None and value not in nxdl_enum: + collector.collect_and_log( + path, + ValidationProblem.InvalidEnum, + nxdl_enum, + ) + + return value @lru_cache(maxsize=None) diff --git a/src/pynxtools/dataconverter/validation.py b/src/pynxtools/dataconverter/validation.py index aade2d9b2..8202a704f 100644 --- a/src/pynxtools/dataconverter/validation.py +++ b/src/pynxtools/dataconverter/validation.py @@ -430,21 +430,13 @@ def handle_field(node: NexusNode, keys: Mapping[str, Any], prev_path: str): continue # Check general validity - _ = is_valid_data_field( - mapping[f"{prev_path}/{variant}"], node.dtype, f"{prev_path}/{variant}" + mapping[f"{prev_path}/{variant}"] = is_valid_data_field( + mapping[f"{prev_path}/{variant}"], + node.dtype, + node.items, + f"{prev_path}/{variant}", ) - # Check enumeration - if ( - node.items is not None - and mapping[f"{prev_path}/{variant}"] not in node.items - ): - collector.collect_and_log( - f"{prev_path}/{variant}", - ValidationProblem.InvalidEnum, - node.items, - ) - # Check unit category if node.unit is not None: remove_from_not_visited(f"{prev_path}/{variant}/@units") @@ -477,28 +469,17 @@ def handle_attribute(node: NexusNode, keys: Mapping[str, Any], prev_path: str): return for variant in variants: - _ = is_valid_data_field( + mapping[ + f"{prev_path}/{variant if variant.startswith('@') else f'@{variant}'}" + ] = is_valid_data_field( mapping[ f"{prev_path}/{variant if variant.startswith('@') else f'@{variant}'}" ], node.dtype, + node.items, f"{prev_path}/{variant if variant.startswith('@') else f'@{variant}'}", ) - # Check enumeration - if ( - node.items is not None - and mapping[ - f"{prev_path}/{variant if variant.startswith('@') else f'@{variant}'}" - ] - not in node.items - ): - collector.collect_and_log( - f"{prev_path}/{variant if variant.startswith('@') else f'@{variant}'}", - ValidationProblem.InvalidEnum, - node.items, - ) - def handle_choice(node: NexusNode, keys: Mapping[str, Any], prev_path: str): global collector old_collector = collector @@ -553,15 +534,7 @@ def is_documented(key: str, node: NexusNode) -> bool: # We still do some further checks before returning. # Check general validity - _ = is_valid_data_field(mapping[key], node.dtype, key) - - # Check enumeration - if node.items is not None and mapping[key] not in node.items: - collector.collect_and_log( - key, - ValidationProblem.InvalidEnum, - node.items, - ) + mapping[key] = is_valid_data_field(mapping[key], node.dtype, node.items, key) # Check main field exists for units if ( From ecb4914e5243e2b906f60671386c88114bdc8482 Mon Sep 17 00:00:00 2001 From: rettigl Date: Fri, 28 Feb 2025 13:18:58 +0100 Subject: [PATCH 16/36] satisfy mypy --- src/pynxtools/dataconverter/validation.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pynxtools/dataconverter/validation.py b/src/pynxtools/dataconverter/validation.py index 8202a704f..ae244e701 100644 --- a/src/pynxtools/dataconverter/validation.py +++ b/src/pynxtools/dataconverter/validation.py @@ -20,7 +20,7 @@ from collections import defaultdict from functools import reduce from operator import getitem -from typing import Any, Iterable, List, Mapping, Optional, Tuple, Union +from typing import Any, Iterable, List, Mapping, MutableMapping, Optional, Tuple, Union import h5py import lxml.etree as ET @@ -165,14 +165,14 @@ def best_namefit_of(name: str, keys: Iterable[str]) -> Optional[str]: def validate_dict_against( - appdef: str, mapping: Mapping[str, Any], ignore_undocumented: bool = False + appdef: str, mapping: MutableMapping[str, Any], ignore_undocumented: bool = False ) -> Tuple[bool, List]: """ Validates a mapping against the NeXus tree for application definition `appdef`. Args: appdef (str): The appdef name to validate against. - mapping (Mapping[str, Any]): + mapping (MutableMapping[str, Any]): The mapping containing the data to validate. This should be a dict of `/` separated paths. Attributes are denoted with `@` in front of the last element. @@ -813,6 +813,6 @@ def populate_full_tree(node: NexusNode, max_depth: Optional[int] = 5, depth: int # Backwards compatibility def validate_data_dict( - _: Mapping[str, Any], read_data: Mapping[str, Any], root: ET._Element + _: MutableMapping[str, Any], read_data: MutableMapping[str, Any], root: ET._Element ) -> bool: return validate_dict_against(root.attrib["name"], read_data)[0] From 4a292517859557dfa92c57ef005106c657d8be2f Mon Sep 17 00:00:00 2001 From: rettigl Date: Tue, 4 Mar 2025 19:28:13 +0100 Subject: [PATCH 17/36] add tests from branch fix_attribute_enum_check --- src/pynxtools/data/NXtest.nxdl.xml | 10 ++ src/pynxtools/dataconverter/helpers.py | 20 ++- .../dataconverter/readers/example/reader.py | 6 +- .../readers/example/testdata.json | 5 +- tests/dataconverter/test_helpers.py | 170 +++++++++++++++--- tests/dataconverter/test_validation.py | 4 + 6 files changed, 178 insertions(+), 37 deletions(-) diff --git a/src/pynxtools/data/NXtest.nxdl.xml b/src/pynxtools/data/NXtest.nxdl.xml index 8695a20c9..45f37896c 100644 --- a/src/pynxtools/data/NXtest.nxdl.xml +++ b/src/pynxtools/data/NXtest.nxdl.xml @@ -28,9 +28,13 @@ + A dummy entry for a float value. + + A dummy entry for a number value. + A dummy entry for a bool value. @@ -53,6 +57,12 @@ + + + + + + diff --git a/src/pynxtools/dataconverter/helpers.py b/src/pynxtools/dataconverter/helpers.py index 307bbbd3d..247ea4880 100644 --- a/src/pynxtools/dataconverter/helpers.py +++ b/src/pynxtools/dataconverter/helpers.py @@ -96,7 +96,7 @@ def _log(self, path: str, log_type: ValidationProblem, value: Optional[Any], *ar ) elif log_type == ValidationProblem.InvalidType: logger.warning( - f"The value at {path} should be one of: {value}" + f"The value at {path} should be one of the following Python types: {value}" f", as defined in the NXDL as {args[0] if args else ''}." ) elif log_type == ValidationProblem.InvalidDatetime: @@ -158,9 +158,9 @@ def collect_and_log( "NX_ANY", ): return - if self.logging: + if self.logging and path + str(log_type) + str(value) not in self.data: self._log(path, log_type, value, *args, **kwargs) - self.data.add(path) + self.data.add(path + str(log_type) + str(value)) def has_validation_problems(self): """Returns True if there were any validation problems.""" @@ -584,7 +584,11 @@ def is_value_valid_element_of_enum(value, elist) -> Tuple[bool, list]: # Not to be confused with `np.byte` and `np.ubyte`, these store # an integer of `8bit` and `unsigned 8bit` respectively. np_bytes = (np.bytes_,) -np_char = (np.str_, np.bytes_) # Only numpy Unicode string and Byte string +np_char = ( + np.str_, + np.bytes_, + np.chararray, +) # Only numpy Unicode string and Byte string np_bool = (np.bool_,) np_complex = (np.complex64, np.complex128, np.cdouble, np.csingle, np.complex_) NEXUS_TO_PYTHON_DATA_TYPES = { @@ -647,9 +651,11 @@ def check_all_children_for_callable( return False if isinstance(objects, list): # Handles list and list of list - return all([type(elem) in accepted_types for elem in objects]) - if isinstance(objects, np.ndarray): - return any([np.issubdtype(objects.dtype, type_) for type_ in accepted_types]) + tmp_arr = np.array(objects) + elif isinstance(objects, np.ndarray): + tmp_arr = objects + if tmp_arr is not None: + return any([np.issubdtype(tmp_arr.dtype, type_) for type_ in accepted_types]) return False diff --git a/src/pynxtools/dataconverter/readers/example/reader.py b/src/pynxtools/dataconverter/readers/example/reader.py index fefe37f5c..7e368a264 100644 --- a/src/pynxtools/dataconverter/readers/example/reader.py +++ b/src/pynxtools/dataconverter/readers/example/reader.py @@ -58,7 +58,11 @@ def read( # outputs with --generate-template for a provided NXDL file if ( k.startswith("/ENTRY[entry]/required_group") - or k == "/ENTRY[entry]/optional_parent/req_group_in_opt_group" + or k + in ( + "/ENTRY[entry]/optional_parent/req_group_in_opt_group", + "/ENTRY[entry]/NXODD_name[nxodd_name]/anamethatRENAMES[anamethatrenames]", + ) or k.startswith("/ENTRY[entry]/OPTIONAL_group") ): continue diff --git a/tests/data/dataconverter/readers/example/testdata.json b/tests/data/dataconverter/readers/example/testdata.json index 21deb40c3..e66af9962 100644 --- a/tests/data/dataconverter/readers/example/testdata.json +++ b/tests/data/dataconverter/readers/example/testdata.json @@ -7,6 +7,8 @@ "float_value_units": "nm", "int_value": -3, "int_value_units": "eV", + "number_value": 3, + "number_value_units": "eV", "posint_value": 7, "posint_value_units": "kg", "definition": "NXtest", @@ -17,5 +19,6 @@ "date_value_units": "", "required_child": 1, "optional_child": 1, - "@version": "1.0" + "@version": "1.0", + "@array": [0, 1, 2] } \ No newline at end of file diff --git a/tests/dataconverter/test_helpers.py b/tests/dataconverter/test_helpers.py index afa8ff71e..c61369489 100644 --- a/tests/dataconverter/test_helpers.py +++ b/tests/dataconverter/test_helpers.py @@ -25,7 +25,6 @@ import numpy as np import pytest - from pynxtools.dataconverter import helpers from pynxtools.dataconverter.template import Template from pynxtools.dataconverter.validation import validate_dict_against @@ -97,9 +96,7 @@ def listify_template(data_dict: Template): "type", "definition", "date_value", - ) or isinstance( - data_dict[optionality][path], np.ndarray - ): # avoid list numpy array + ) or isinstance(data_dict[optionality][path], list): listified_template[optionality][path] = data_dict[optionality][path] else: listified_template[optionality][path] = [data_dict[optionality][path]] @@ -158,6 +155,9 @@ def fixture_filled_test_data(template, tmp_path): ) template.clear() + template[ + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/anamethatRENAMES[anamethatichangetothis]" + ] = 2 template["/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value"] = 2.0 template["/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value/@units"] = "nm" template["/ENTRY[my_entry]/optional_parent/required_child"] = 1 @@ -165,6 +165,8 @@ def fixture_filled_test_data(template, tmp_path): template["/ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value"] = True template["/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value"] = 2 template["/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value/@units"] = "eV" + template["/ENTRY[my_entry]/NXODD_name[nxodd_name]/number_value"] = 2 + template["/ENTRY[my_entry]/NXODD_name[nxodd_name]/number_value/@units"] = "eV" template["/ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value"] = np.array( [1, 2, 3], dtype=np.int8 ) @@ -187,6 +189,9 @@ def fixture_filled_test_data(template, tmp_path): TEMPLATE = Template() +TEMPLATE["optional"][ + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/anamethatRENAMES[anamethatichangetothis]" +] = 2 TEMPLATE["optional"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value"] = 2.0 # pylint: disable=E1126 TEMPLATE["optional"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value/@units"] = ( "nm" # pylint: disable=E1126 @@ -197,6 +202,10 @@ def fixture_filled_test_data(template, tmp_path): TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value/@units"] = "" TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value"] = 2 # pylint: disable=E1126 TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value/@units"] = "eV" # pylint: disable=E1126 +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/number_value"] = 2 +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/number_value/@units"] = ( + "eV" +) TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value"] = np.array( [1, 2, 3], # pylint: disable=E1126 dtype=np.int8, @@ -212,6 +221,9 @@ def fixture_filled_test_data(template, tmp_path): TEMPLATE["required"][ "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/bool_value/@units" ] = "" +TEMPLATE["required"][ + "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/anamethatRENAMES[anamethatichangetothis]" +] = 2 # pylint: disable=E1126 TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/int_value"] = 2 # pylint: disable=E1126 TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/int_value/@units"] = ( "eV" # pylint: disable=E1126 @@ -232,6 +244,11 @@ def fixture_filled_test_data(template, tmp_path): "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/char_value/@units" ] = "" TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/type"] = "2nd type" # pylint: disable=E1126 +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/type/@array"] = [ + 0, + 1, + 2, +] TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/date_value"] = ( "2022-01-22T12:14:12.05018+00:00" # pylint: disable=E1126 ) @@ -243,6 +260,7 @@ def fixture_filled_test_data(template, tmp_path): TEMPLATE["required"]["/ENTRY[my_entry]/definition/@version"] = "2.4.6" # pylint: disable=E1126 TEMPLATE["required"]["/ENTRY[my_entry]/program_name"] = "Testing program" # pylint: disable=E1126 TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/type"] = "2nd type" # pylint: disable=E1126 +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/type/@array"] = [0, 1, 2] TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/date_value"] = ( "2022-01-22T12:14:12.05018+00:00" # pylint: disable=E1126 ) @@ -274,6 +292,19 @@ def fixture_filled_test_data(template, tmp_path): @pytest.mark.parametrize( "data_dict,error_message", [ + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/anamethatRENAMES[anamethatichangetothis]", + "not_a_num", + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/anamethatRENAMES[anamethatichangetothis]" + " should be one of the following Python types: (, ), as defined in " + "the NXDL as NX_INT." + ), + id="variadic-field-str-instead-of-int", + ), pytest.param( alter_dict( TEMPLATE, @@ -281,12 +312,23 @@ def fixture_filled_test_data(template, tmp_path): "not_a_num", ), ( - " The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value should" - " be one of: (, ), as defined in the" - " NXDL as NX_INT.\n" + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/in" + "t_value should be one of the following Python types: (, ), as defined in " + "the NXDL as NX_INT." ), id="string-instead-of-int", ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value", + "NOT_TRUE_OR_FALSE", + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value should be one of the following Python types: (, ), as defined in the NXDL as NX_BOOLEAN." + ), + id="string-instead-of-bool", + ), pytest.param( alter_dict( TEMPLATE, @@ -294,8 +336,8 @@ def fixture_filled_test_data(template, tmp_path): ["1", "2", "3"], ), ( - " The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value should" - " be one of: (, )" + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value should" + " be one of the following Python types: (, ), as defined in the NXDL as NX_INT." ), id="list-of-int-str-instead-of-int", ), @@ -307,7 +349,7 @@ def fixture_filled_test_data(template, tmp_path): ), ( "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value should be" - " one of: (, )" + " one of the following Python types: (, ), as defined in the NXDL as NX_INT." ), id="array-of-float-instead-of-int", ), @@ -318,7 +360,7 @@ def fixture_filled_test_data(template, tmp_path): [2, 3, 4], ), (""), - id="List-of-int-instead-of-int", + id="list-of-int-instead-of-int", ), pytest.param( alter_dict( @@ -332,15 +374,58 @@ def fixture_filled_test_data(template, tmp_path): pytest.param( alter_dict( TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value", - "NOT_TRUE_OR_FALSE", + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/date_value", + "2022-01-22T12:14:12.05018-00:00", + ), + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/date_value" + " = 2022-01-22T12:14:12.05018-00:00 should be a timezone aware" + " ISO8601 formatted str. For example, 2022-01-22T12:14:12.05018Z or 2022-01-22" + "T12:14:12.05018+00:00.", + id="int-instead-of-date", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value", + 0, ), ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value should " - "be one of: (, ), as defined in the " - "NXDL as NX_BOOLEAN" + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value should be one of the following Python types: (, ), as defined in the NXDL as NX_FLOAT." ), - id="string-instead-of-bool", + id="int-instead-of-float", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/number_value", + "0", + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/number_value should be one of the following Python types: (, , , ), as defined in the NXDL as NX_NUMBER." + ), + id="str-instead-of-number", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", + np.array([0.0, 2]), + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value should be one" + " of the following Python types: (, , , ), as" + " defined in the NXDL as NX_CHAR." + ), + id="wrong-type-ndarray-instead-of-char", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", + np.array(["x", "2"]), + ), + (""), + id="valid-ndarray-instead-of-char", ), pytest.param( alter_dict( @@ -408,9 +493,9 @@ def fixture_filled_test_data(template, tmp_path): TEMPLATE, "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", 3 ), ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value should" - " be one of: (, , " - "), as defined in the NXDL as NX_CHAR." + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value should be one of the following Python types:" + " (, , , )," + " as defined in the NXDL as NX_CHAR." ), id="int-instead-of-chars", ), @@ -475,8 +560,8 @@ def fixture_filled_test_data(template, tmp_path): np.array(["2.0", "3.0"], dtype=np.str_), ), "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value should be " - "one of: (, ), as defined in the NXDL " - "as NX_FLOAT.\n", + "one of the following Python types: (, ), as defined in the NXDL " + "as NX_FLOAT.", id="array-of-str-instead-of-float", ), pytest.param( @@ -485,9 +570,9 @@ def fixture_filled_test_data(template, tmp_path): "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value", [2], # pylint: disable=E1126 ), - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value should be" - " one of: (, ), as defined in the NXDL" - " as NX_FLOAT.\n", + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value should be " + "one of the following Python types: (, ), as defined in the NXDL " + "as NX_FLOAT.", id="list-of-int-instead-of-float", ), pytest.param( @@ -669,10 +754,34 @@ def fixture_filled_test_data(template, tmp_path): pytest.param( remove_optional_parent(TEMPLATE), (""), id="opt-group-completely-removed" ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/type/@array", + ["0", 1, 2], + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/type/@array should be one of the following: [[0, 1, 2], [2, 3, 4]]" + ), + id="wrong-type-array-in-attribute", + ), + pytest.param( + alter_dict( + TEMPLATE, "/ENTRY[my_entry]/NXODD_name[nxodd_name]/type/@array", [1, 2] + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/type/@array should be one of the following: [[0, 1, 2], [2, 3, 4]]" + ), + id="wrong-value-array-in-attribute", + ), ], ) def test_validate_data_dict(caplog, data_dict, error_message, request): """Unit test for the data validation routine.""" + + def format_error_message(msg: str) -> str: + return msg[msg.rfind("G: ") + 3 :].rstrip("\n") + if request.node.callspec.id in ( "valid-data-dict", "lists", @@ -683,6 +792,8 @@ def test_validate_data_dict(caplog, data_dict, error_message, request): "link-dict-instead-of-int", "opt-group-completely-removed", "required-field-provided-in-variadic-optional-group", + "valid-ndarray-instead-of-char", + "list-of-int-instead-of-int", "list-of-string-instead-of-chars", "array-of-int32-instead-of-int", "List-of-int-instead-of-int", @@ -706,12 +817,15 @@ def test_validate_data_dict(caplog, data_dict, error_message, request): assert "" == caplog.text captured_logs = caplog.records assert not validate_dict_against("NXtest", data_dict)[0] - assert any(error_message in rec.message for rec in captured_logs) + assert any( + error_message == format_error_message(rec.message) for rec in captured_logs + ) else: with caplog.at_level(logging.WARNING): assert not validate_dict_against("NXtest", data_dict)[0] - - assert error_message in caplog.text + assert any( + error_message == format_error_message(rec.message) for rec in caplog.records + ) @pytest.mark.parametrize( diff --git a/tests/dataconverter/test_validation.py b/tests/dataconverter/test_validation.py index 2c946a3a1..8bb892998 100644 --- a/tests/dataconverter/test_validation.py +++ b/tests/dataconverter/test_validation.py @@ -28,6 +28,7 @@ def get_data_dict(): return { "/ENTRY[my_entry]/optional_parent/required_child": 1, "/ENTRY[my_entry]/optional_parent/optional_child": 1, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/anamethatRENAMES[anamethatichangetothis]": 2, "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value_no_attr": 2.0, "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value": 2.0, "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value/@units": "nm", @@ -42,8 +43,10 @@ def get_data_dict(): "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value": "just chars", "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value/@units": "", "/ENTRY[my_entry]/NXODD_name[nxodd_name]/type": "2nd type", + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/type/@array": [0, 1, 2], "/ENTRY[my_entry]/NXODD_name[nxodd_name]/date_value": "2022-01-22T12:14:12.05018+00:00", "/ENTRY[my_entry]/NXODD_name[nxodd_name]/date_value/@units": "", + "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/anamethatRENAMES[anamethatichangetothis]": 2, "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/bool_value": True, "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/bool_value/@units": "", "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/int_value": 2, @@ -55,6 +58,7 @@ def get_data_dict(): "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/char_value": "just chars", "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/char_value/@units": "", "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/type": "2nd type", + "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/type/@array": [0, 1, 2], "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/date_value": "2022-01-22T12:14:12.05018+00:00", "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/date_value/@units": "", "/ENTRY[my_entry]/OPTIONAL_group[my_group]/required_field": 1, From 31cd1312a2a9177fd0b04477fe6cf86388233c8e Mon Sep 17 00:00:00 2001 From: rettigl Date: Wed, 5 Mar 2025 09:19:21 +0100 Subject: [PATCH 18/36] add review suggestion --- src/pynxtools/data/NXtest.nxdl.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pynxtools/data/NXtest.nxdl.xml b/src/pynxtools/data/NXtest.nxdl.xml index 45f37896c..2d6547698 100644 --- a/src/pynxtools/data/NXtest.nxdl.xml +++ b/src/pynxtools/data/NXtest.nxdl.xml @@ -28,7 +28,7 @@ - + A dummy entry for a float value. From 3aabe0b4ca485570063ea28b61407ca500f5ae86 Mon Sep 17 00:00:00 2001 From: Sherjeel Shabih Date: Wed, 5 Mar 2025 11:34:34 +0100 Subject: [PATCH 19/36] Fixes the types and removes bytes from NX_char as that creates failures --- src/pynxtools/dataconverter/helpers.py | 71 +++++++------------------- tests/dataconverter/test_helpers.py | 22 +++----- 2 files changed, 24 insertions(+), 69 deletions(-) diff --git a/src/pynxtools/dataconverter/helpers.py b/src/pynxtools/dataconverter/helpers.py index 247ea4880..98cf6c4e6 100644 --- a/src/pynxtools/dataconverter/helpers.py +++ b/src/pynxtools/dataconverter/helpers.py @@ -575,60 +575,25 @@ def is_value_valid_element_of_enum(value, elist) -> Tuple[bool, list]: return True, [] -NUMPY_FLOAT_TYPES = (np.half, np.float16, np.single, np.double, np.longdouble) -NUMPY_INT_TYPES = (np.short, np.intc, np.int_) -NUMPY_UINT_TYPES = (np.ushort, np.uintc, np.uint) -# np int for np version 1.26.0 -np_int = (np.integer,) -np_float = (np.floating,) -# Not to be confused with `np.byte` and `np.ubyte`, these store -# an integer of `8bit` and `unsigned 8bit` respectively. -np_bytes = (np.bytes_,) -np_char = ( - np.str_, - np.bytes_, - np.chararray, -) # Only numpy Unicode string and Byte string -np_bool = (np.bool_,) -np_complex = (np.complex64, np.complex128, np.cdouble, np.csingle, np.complex_) +nx_char = (str, np.chararray) +nx_int = (int, np.integer) +nx_float = (float, np.floating) +nx_number = nx_int + nx_float + NEXUS_TO_PYTHON_DATA_TYPES = { - "ISO8601": (str,), - "NX_BINARY": ( - bytes, - *np_bytes, - ), - "NX_BOOLEAN": (bool, *np_bool), - "NX_CHAR": (str, *np_char), - "NX_DATE_TIME": (str,), - "NX_FLOAT": (float, *np_float), - "NX_INT": (int, *np_int), - "NX_UINT": ( - np.unsignedinteger, - np.uint, - ), - "NX_NUMBER": ( - int, - float, - *np_int, - *np_float, - ), - "NX_POSINT": ( - int, - np.signedinteger, - ), # > 0 is checked in is_valid_data_field() - "NX_COMPLEX": (complex, *np_complex), - "NXDL_TYPE_UNAVAILABLE": ( - str, - *np_char, - ), # Defaults to a string if a type is not provided. - "NX_CHAR_OR_NUMBER": ( - str, - int, - float, - *np_char, - *np_int, - *np_float, - ), + "ISO8601": (str), + "NX_BINARY": (bytes, bytearray, np.byte, np.ubyte), + "NX_BOOLEAN": (bool, np.bool_), + "NX_CHAR": nx_char, + "NX_DATE_TIME": (str), + "NX_FLOAT": nx_float, + "NX_INT": nx_int, + "NX_UINT": (np.unsignedinteger), + "NX_NUMBER": nx_number, + "NX_POSINT": nx_int, # > 0 is checked in is_valid_data_field() + "NX_COMPLEX": (complex, np.complexfloating), + "NX_CHAR_OR_NUMBER": nx_char + nx_number, + "NXDL_TYPE_UNAVAILABLE": (str), # Defaults to a string if a type is not provided. } diff --git a/tests/dataconverter/test_helpers.py b/tests/dataconverter/test_helpers.py index c61369489..a14b57bb5 100644 --- a/tests/dataconverter/test_helpers.py +++ b/tests/dataconverter/test_helpers.py @@ -401,7 +401,7 @@ def fixture_filled_test_data(template, tmp_path): "0", ), ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/number_value should be one of the following Python types: (, , , ), as defined in the NXDL as NX_NUMBER." + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/number_value should be one of the following Python types: (, , , ), as defined in the NXDL as NX_NUMBER." ), id="str-instead-of-number", ), @@ -413,7 +413,7 @@ def fixture_filled_test_data(template, tmp_path): ), ( "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value should be one" - " of the following Python types: (, , , ), as" + " of the following Python types: (, ), as" " defined in the NXDL as NX_CHAR." ), id="wrong-type-ndarray-instead-of-char", @@ -494,7 +494,7 @@ def fixture_filled_test_data(template, tmp_path): ), ( "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value should be one of the following Python types:" - " (, , , )," + " (, )," " as defined in the NXDL as NX_CHAR." ), id="int-instead-of-chars", @@ -512,22 +512,11 @@ def fixture_filled_test_data(template, tmp_path): alter_dict( TEMPLATE, "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", - np.array(["1", "2", "3"], dtype=np.bytes_), + np.char.array(["1", "2", "3"]), ), (""), - id="array-of-bytes-chars", + id="numpy-chararray", ), - # pytest.param( - # alter_dict( - # TEMPLATE, - # "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", - # np.char.chararray(["1", "2", "3"]), - # ), - # (""), - # id="numpy-chararray", - # ), - # TODO add test array of char - # TODO add test for numpy array of char and chararray pytest.param( alter_dict( TEMPLATE, @@ -802,6 +791,7 @@ def format_error_message(msg: str) -> str: "array-of-chars", "array-of-bytes-chars", "array-of-float-instead-of-float", + "numpy-chararray", ): with caplog.at_level(logging.WARNING): assert validate_dict_against("NXtest", data_dict)[0] From bd478e89e41b2bf8e95df98a5e0eff5601f2fcd3 Mon Sep 17 00:00:00 2001 From: Sherjeel Shabih Date: Wed, 5 Mar 2025 12:52:08 +0100 Subject: [PATCH 20/36] Fixes for arrays in an array --- src/pynxtools/dataconverter/helpers.py | 39 ++++++-------------------- 1 file changed, 8 insertions(+), 31 deletions(-) diff --git a/src/pynxtools/dataconverter/helpers.py b/src/pynxtools/dataconverter/helpers.py index 98cf6c4e6..019bdc0b4 100644 --- a/src/pynxtools/dataconverter/helpers.py +++ b/src/pynxtools/dataconverter/helpers.py @@ -24,7 +24,7 @@ from datetime import datetime, timezone from enum import Enum from functools import lru_cache -from typing import Any, Callable, List, Optional, Tuple, Union +from typing import Any, Callable, List, Optional, Tuple, Union, Sequence import h5py import lxml.etree as ET @@ -598,52 +598,29 @@ def is_value_valid_element_of_enum(value, elist) -> Tuple[bool, list]: def check_all_children_for_callable( - objects: Union[list, np.ndarray], - checker: Optional[Callable] = None, - accepted_types: Optional[tuple] = None, + objects: Union[list, np.ndarray], checker: Optional[Callable] = None, *args ) -> bool: """Checks whether all objects in list or numpy array are validated by given callable and types. """ + if not isinstance(objects, np.ndarray): + objects = np.array(objects) - if checker is not None: - for obj in objects: - args = (obj, accepted_types) if accepted_types is not None else (obj,) - if not checker(*args): - return False - return True - if isinstance(objects, tuple): - return False - if isinstance(objects, list): - # Handles list and list of list - tmp_arr = np.array(objects) - elif isinstance(objects, np.ndarray): - tmp_arr = objects - if tmp_arr is not None: - return any([np.issubdtype(tmp_arr.dtype, type_) for type_ in accepted_types]) - - return False + return all([checker(o, *args) for o in objects.flat]) def is_valid_data_type(value, accepted_types): """Checks whether the given value or its children are of an accepted type.""" - - if not isinstance(value, (list, np.ndarray)): - return isinstance(value, accepted_types) - - return check_all_children_for_callable(objects=value, accepted_types=accepted_types) + return check_all_children_for_callable(value, isinstance, accepted_types) def is_positive_int(value): """Checks whether the given value or its children are positive.""" def is_greater_than(num): - return num.flat[0] > 0 if isinstance(num, np.ndarray) else num > 0 - - if isinstance(value, list): - return check_all_children_for_callable(objects=value, checker=is_greater_than) + return num > 0 - return value.flat[0] > 0 if isinstance(value, np.ndarray) else value > 0 + return check_all_children_for_callable(objects=value, checker=is_greater_than) def convert_str_to_bool_safe(value: str) -> Optional[bool]: From b336ca54e462446d486624273cef7eabcfba5f07 Mon Sep 17 00:00:00 2001 From: rettigl Date: Wed, 5 Mar 2025 14:00:29 +0100 Subject: [PATCH 21/36] fix mypy error --- src/pynxtools/dataconverter/helpers.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/pynxtools/dataconverter/helpers.py b/src/pynxtools/dataconverter/helpers.py index 019bdc0b4..fbe7b741a 100644 --- a/src/pynxtools/dataconverter/helpers.py +++ b/src/pynxtools/dataconverter/helpers.py @@ -581,19 +581,19 @@ def is_value_valid_element_of_enum(value, elist) -> Tuple[bool, list]: nx_number = nx_int + nx_float NEXUS_TO_PYTHON_DATA_TYPES = { - "ISO8601": (str), + "ISO8601": (str,), "NX_BINARY": (bytes, bytearray, np.byte, np.ubyte), "NX_BOOLEAN": (bool, np.bool_), "NX_CHAR": nx_char, - "NX_DATE_TIME": (str), + "NX_DATE_TIME": (str,), "NX_FLOAT": nx_float, "NX_INT": nx_int, - "NX_UINT": (np.unsignedinteger), + "NX_UINT": (np.unsignedinteger,), "NX_NUMBER": nx_number, "NX_POSINT": nx_int, # > 0 is checked in is_valid_data_field() - "NX_COMPLEX": (complex, np.complexfloating), + "NX_COMPLEX": (complex, np.complexfloating,), "NX_CHAR_OR_NUMBER": nx_char + nx_number, - "NXDL_TYPE_UNAVAILABLE": (str), # Defaults to a string if a type is not provided. + "NXDL_TYPE_UNAVAILABLE": (str,), # Defaults to a string if a type is not provided. } From e9b025e8d1f01b403d33bd2310441054a6acea63 Mon Sep 17 00:00:00 2001 From: rettigl Date: Wed, 5 Mar 2025 14:32:28 +0100 Subject: [PATCH 22/36] ruff --- src/pynxtools/dataconverter/helpers.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/pynxtools/dataconverter/helpers.py b/src/pynxtools/dataconverter/helpers.py index fbe7b741a..aefe9a804 100644 --- a/src/pynxtools/dataconverter/helpers.py +++ b/src/pynxtools/dataconverter/helpers.py @@ -591,7 +591,10 @@ def is_value_valid_element_of_enum(value, elist) -> Tuple[bool, list]: "NX_UINT": (np.unsignedinteger,), "NX_NUMBER": nx_number, "NX_POSINT": nx_int, # > 0 is checked in is_valid_data_field() - "NX_COMPLEX": (complex, np.complexfloating,), + "NX_COMPLEX": ( + complex, + np.complexfloating, + ), "NX_CHAR_OR_NUMBER": nx_char + nx_number, "NXDL_TYPE_UNAVAILABLE": (str,), # Defaults to a string if a type is not provided. } From 8c89eef432ab2b75e3ee221ec25b4b6f5d270571 Mon Sep 17 00:00:00 2001 From: Sherjeel Shabih Date: Tue, 11 Mar 2025 09:16:05 +0100 Subject: [PATCH 23/36] Applies suggested fix Co-authored-by: Laurenz Rettig <53396064+rettigl@users.noreply.github.com> --- src/pynxtools/dataconverter/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pynxtools/dataconverter/helpers.py b/src/pynxtools/dataconverter/helpers.py index aefe9a804..9dc1770bc 100644 --- a/src/pynxtools/dataconverter/helpers.py +++ b/src/pynxtools/dataconverter/helpers.py @@ -575,7 +575,7 @@ def is_value_valid_element_of_enum(value, elist) -> Tuple[bool, list]: return True, [] -nx_char = (str, np.chararray) +nx_char = (str, np.character) nx_int = (int, np.integer) nx_float = (float, np.floating) nx_number = nx_int + nx_float From 1c5538da115cc998d04544ebcd323e4cf03d14d0 Mon Sep 17 00:00:00 2001 From: Sherjeel Shabih Date: Tue, 11 Mar 2025 09:16:27 +0100 Subject: [PATCH 24/36] Update src/pynxtools/dataconverter/helpers.py Co-authored-by: Laurenz Rettig <53396064+rettigl@users.noreply.github.com> --- src/pynxtools/dataconverter/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pynxtools/dataconverter/helpers.py b/src/pynxtools/dataconverter/helpers.py index 9dc1770bc..0278faead 100644 --- a/src/pynxtools/dataconverter/helpers.py +++ b/src/pynxtools/dataconverter/helpers.py @@ -596,7 +596,7 @@ def is_value_valid_element_of_enum(value, elist) -> Tuple[bool, list]: np.complexfloating, ), "NX_CHAR_OR_NUMBER": nx_char + nx_number, - "NXDL_TYPE_UNAVAILABLE": (str,), # Defaults to a string if a type is not provided. + "NXDL_TYPE_UNAVAILABLE": (nx_char,), # Defaults to a string if a type is not provided. } From 365f061347f7501a264712b04420979abf7dc40a Mon Sep 17 00:00:00 2001 From: Sherjeel Shabih Date: Tue, 11 Mar 2025 09:28:44 +0100 Subject: [PATCH 25/36] Applies fixes from suggestions --- src/pynxtools/dataconverter/helpers.py | 6 +++--- tests/dataconverter/test_helpers.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pynxtools/dataconverter/helpers.py b/src/pynxtools/dataconverter/helpers.py index 0278faead..3fe1c58d7 100644 --- a/src/pynxtools/dataconverter/helpers.py +++ b/src/pynxtools/dataconverter/helpers.py @@ -582,7 +582,7 @@ def is_value_valid_element_of_enum(value, elist) -> Tuple[bool, list]: NEXUS_TO_PYTHON_DATA_TYPES = { "ISO8601": (str,), - "NX_BINARY": (bytes, bytearray, np.byte, np.ubyte), + "NX_BINARY": (bytes, bytearray, np.bytes_), "NX_BOOLEAN": (bool, np.bool_), "NX_CHAR": nx_char, "NX_DATE_TIME": (str,), @@ -601,7 +601,7 @@ def is_value_valid_element_of_enum(value, elist) -> Tuple[bool, list]: def check_all_children_for_callable( - objects: Union[list, np.ndarray], checker: Optional[Callable] = None, *args + objects: Union[list, np.ndarray], check_function: Optional[Callable] = None, *args ) -> bool: """Checks whether all objects in list or numpy array are validated by given callable and types. @@ -609,7 +609,7 @@ def check_all_children_for_callable( if not isinstance(objects, np.ndarray): objects = np.array(objects) - return all([checker(o, *args) for o in objects.flat]) + return all([check_function(o, *args) for o in objects.flat]) def is_valid_data_type(value, accepted_types): diff --git a/tests/dataconverter/test_helpers.py b/tests/dataconverter/test_helpers.py index a14b57bb5..310dfa6e2 100644 --- a/tests/dataconverter/test_helpers.py +++ b/tests/dataconverter/test_helpers.py @@ -512,7 +512,7 @@ def fixture_filled_test_data(template, tmp_path): alter_dict( TEMPLATE, "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", - np.char.array(["1", "2", "3"]), + np.array(["1", "2", "3"], dtype=np.bytes_), ), (""), id="numpy-chararray", From bd250cc2ef65bcbdbe9f71f78bc894a6b66f7c3c Mon Sep 17 00:00:00 2001 From: Sherjeel Shabih Date: Tue, 11 Mar 2025 09:31:22 +0100 Subject: [PATCH 26/36] Updates --- tests/dataconverter/test_helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/dataconverter/test_helpers.py b/tests/dataconverter/test_helpers.py index 310dfa6e2..502bc114a 100644 --- a/tests/dataconverter/test_helpers.py +++ b/tests/dataconverter/test_helpers.py @@ -515,7 +515,7 @@ def fixture_filled_test_data(template, tmp_path): np.array(["1", "2", "3"], dtype=np.bytes_), ), (""), - id="numpy-chararray", + id="array-of-bytes-chars", ), pytest.param( alter_dict( From 84426b7dda729e0585df6d9e3ecc21ccab693b06 Mon Sep 17 00:00:00 2001 From: Sherjeel Shabih Date: Tue, 11 Mar 2025 09:34:17 +0100 Subject: [PATCH 27/36] Ruff --- src/pynxtools/dataconverter/helpers.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/pynxtools/dataconverter/helpers.py b/src/pynxtools/dataconverter/helpers.py index 3fe1c58d7..ada958a24 100644 --- a/src/pynxtools/dataconverter/helpers.py +++ b/src/pynxtools/dataconverter/helpers.py @@ -596,7 +596,9 @@ def is_value_valid_element_of_enum(value, elist) -> Tuple[bool, list]: np.complexfloating, ), "NX_CHAR_OR_NUMBER": nx_char + nx_number, - "NXDL_TYPE_UNAVAILABLE": (nx_char,), # Defaults to a string if a type is not provided. + "NXDL_TYPE_UNAVAILABLE": ( + nx_char, + ), # Defaults to a string if a type is not provided. } From 5a63c3b5a53a76cb2e8998fa2cd0f3bdd27a5cc4 Mon Sep 17 00:00:00 2001 From: Sherjeel Shabih Date: Tue, 11 Mar 2025 09:56:02 +0100 Subject: [PATCH 28/36] Update src/pynxtools/dataconverter/helpers.py Co-authored-by: Laurenz Rettig <53396064+rettigl@users.noreply.github.com> --- src/pynxtools/dataconverter/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pynxtools/dataconverter/helpers.py b/src/pynxtools/dataconverter/helpers.py index ada958a24..8699e37c6 100644 --- a/src/pynxtools/dataconverter/helpers.py +++ b/src/pynxtools/dataconverter/helpers.py @@ -625,7 +625,7 @@ def is_positive_int(value): def is_greater_than(num): return num > 0 - return check_all_children_for_callable(objects=value, checker=is_greater_than) + return check_all_children_for_callable(objects=value, check_function=is_greater_than) def convert_str_to_bool_safe(value: str) -> Optional[bool]: From 0d056b7bebd46fa139bdf6f660faf3bb2b7ff797 Mon Sep 17 00:00:00 2001 From: Sherjeel Shabih Date: Tue, 11 Mar 2025 10:40:34 +0100 Subject: [PATCH 29/36] ruff --- src/pynxtools/dataconverter/helpers.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/pynxtools/dataconverter/helpers.py b/src/pynxtools/dataconverter/helpers.py index 8699e37c6..030e5c92d 100644 --- a/src/pynxtools/dataconverter/helpers.py +++ b/src/pynxtools/dataconverter/helpers.py @@ -625,7 +625,9 @@ def is_positive_int(value): def is_greater_than(num): return num > 0 - return check_all_children_for_callable(objects=value, check_function=is_greater_than) + return check_all_children_for_callable( + objects=value, check_function=is_greater_than + ) def convert_str_to_bool_safe(value: str) -> Optional[bool]: From 70a457cf60e397b35a1f6979e8123a451e1132d4 Mon Sep 17 00:00:00 2001 From: rettigl Date: Tue, 11 Mar 2025 11:17:37 +0100 Subject: [PATCH 30/36] remove empty string --- src/pynxtools/dataconverter/helpers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pynxtools/dataconverter/helpers.py b/src/pynxtools/dataconverter/helpers.py index 030e5c92d..9813e8898 100644 --- a/src/pynxtools/dataconverter/helpers.py +++ b/src/pynxtools/dataconverter/helpers.py @@ -215,7 +215,6 @@ def get_nxdl_name_for(xml_elem: ET._Element) -> Optional[str]: The name of the element. None if the xml element has no name or type attribute. """ - """""" if "name" in xml_elem.attrib: return xml_elem.attrib["name"] if "type" in xml_elem.attrib: From b6b112b49823093edd5d01eddd8f303aeac345cd Mon Sep 17 00:00:00 2001 From: Sherjeel Shabih Date: Tue, 11 Mar 2025 12:13:37 +0100 Subject: [PATCH 31/36] Fixes --- tests/dataconverter/test_helpers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/dataconverter/test_helpers.py b/tests/dataconverter/test_helpers.py index 502bc114a..c59467255 100644 --- a/tests/dataconverter/test_helpers.py +++ b/tests/dataconverter/test_helpers.py @@ -413,7 +413,7 @@ def fixture_filled_test_data(template, tmp_path): ), ( "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value should be one" - " of the following Python types: (, ), as" + " of the following Python types: (, ), as" " defined in the NXDL as NX_CHAR." ), id="wrong-type-ndarray-instead-of-char", @@ -494,7 +494,7 @@ def fixture_filled_test_data(template, tmp_path): ), ( "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value should be one of the following Python types:" - " (, )," + " (, )," " as defined in the NXDL as NX_CHAR." ), id="int-instead-of-chars", From 7892971ad70adb9735d22ab5f65bb39d861542b7 Mon Sep 17 00:00:00 2001 From: rettigl Date: Tue, 11 Mar 2025 20:01:30 +0100 Subject: [PATCH 32/36] move validations tests to test_validation --- tests/dataconverter/test_helpers.py | 731 +------------------------ tests/dataconverter/test_validation.py | 712 +++++++++++++++++++++++- 2 files changed, 716 insertions(+), 727 deletions(-) diff --git a/tests/dataconverter/test_helpers.py b/tests/dataconverter/test_helpers.py index c59467255..b01ba9e3c 100644 --- a/tests/dataconverter/test_helpers.py +++ b/tests/dataconverter/test_helpers.py @@ -30,18 +30,6 @@ from pynxtools.dataconverter.validation import validate_dict_against -def remove_optional_parent(data_dict: Template): - """Completely removes the optional group from the test Template.""" - internal_dict = Template(data_dict) - del internal_dict["/ENTRY[my_entry]/optional_parent/required_child"] - del internal_dict["/ENTRY[my_entry]/optional_parent/optional_child"] - del internal_dict[ - "/ENTRY[my_entry]/optional_parent/req_group_in_opt_group/DATA[data]" - ] - - return internal_dict - - def alter_dict(data_dict: Template, key: str, value: object): """Helper function to alter a single entry in dict for parametrize.""" if data_dict is not None: @@ -52,83 +40,6 @@ def alter_dict(data_dict: Template, key: str, value: object): return None -def set_to_none_in_dict(data_dict: Optional[Template], key: str, optionality: str): - """Helper function to forcefully set path to 'None'""" - if data_dict is None: - return None - - internal_dict = Template(data_dict) - internal_dict[optionality][key] = None - return internal_dict - - -def set_whole_group_to_none( - data_dict: Optional[Template], key: str, optionality: str -) -> Optional[Template]: - """Set a whole path to None in the dict""" - if data_dict is None: - return None - - internal_dict = Template(data_dict) - for path in data_dict[optionality]: - if path.startswith(key): - internal_dict[optionality][path] = None - return internal_dict - - -def remove_from_dict(data_dict: Template, key: str, optionality: str = "optional"): - """Helper function to remove a key from dict""" - if data_dict is not None and key in data_dict[optionality]: - internal_dict = Template(data_dict) - del internal_dict[optionality][key] - return internal_dict - - return None - - -def listify_template(data_dict: Template): - """Helper function to turn most values in the Template into lists""" - listified_template = Template() - for optionality in ("optional", "recommended", "required", "undocumented"): - for path in data_dict[optionality]: - if path[path.rindex("/") + 1 :] in ( - "@units", - "type", - "definition", - "date_value", - ) or isinstance(data_dict[optionality][path], list): - listified_template[optionality][path] = data_dict[optionality][path] - else: - listified_template[optionality][path] = [data_dict[optionality][path]] - return listified_template - - -@pytest.mark.parametrize( - "input_data, expected_output", - [ - ("2.4E-23", 2.4e-23), - ("28", 28), - ("45.98", 45.98), - ("test", "test"), - (["59", "3.00005", "498E-36"], np.array([59.0, 3.00005, 4.98e-34])), - ("23 34 444 5000", np.array([23.0, 34.0, 444.0, 5000.0])), - ("xrd experiment", "xrd experiment"), - (None, None), - ], -) -def test_transform_to_intended_dt(input_data, expected_output): - """Transform to possible numerical method.""" - result = helpers.transform_to_intended_dt(input_data) - - # Use pytest.approx for comparing floating-point numbers - if isinstance(expected_output, np.ndarray): - np.testing.assert_allclose(result, expected_output, rtol=1e-3) - elif isinstance(expected_output, float): - assert result == pytest.approx(expected_output, rel=1e-5) - else: - assert result == expected_output - - @pytest.fixture(name="template") def fixture_template(): """pytest fixture to use the same template in all tests""" @@ -188,634 +99,30 @@ def fixture_filled_test_data(template, tmp_path): return template -TEMPLATE = Template() -TEMPLATE["optional"][ - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/anamethatRENAMES[anamethatichangetothis]" -] = 2 -TEMPLATE["optional"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value"] = 2.0 # pylint: disable=E1126 -TEMPLATE["optional"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value/@units"] = ( - "nm" # pylint: disable=E1126 -) -TEMPLATE["optional"]["/ENTRY[my_entry]/optional_parent/required_child"] = 1 # pylint: disable=E1126 -TEMPLATE["optional"]["/ENTRY[my_entry]/optional_parent/optional_child"] = 1 # pylint: disable=E1126 -TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value"] = True # pylint: disable=E1126 -TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value/@units"] = "" -TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value"] = 2 # pylint: disable=E1126 -TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value/@units"] = "eV" # pylint: disable=E1126 -TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/number_value"] = 2 -TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/number_value/@units"] = ( - "eV" -) -TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value"] = np.array( - [1, 2, 3], # pylint: disable=E1126 - dtype=np.int8, -) # pylint: disable=E1126 -TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value/@units"] = ( - "kg" # pylint: disable=E1126 -) -TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value"] = ( - "just chars" # pylint: disable=E1126 -) -TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value/@units"] = "" -TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/bool_value"] = True # pylint: disable=E1126 -TEMPLATE["required"][ - "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/bool_value/@units" -] = "" -TEMPLATE["required"][ - "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/anamethatRENAMES[anamethatichangetothis]" -] = 2 # pylint: disable=E1126 -TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/int_value"] = 2 # pylint: disable=E1126 -TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/int_value/@units"] = ( - "eV" # pylint: disable=E1126 -) -TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/posint_value"] = ( - np.array( - [1, 2, 3], # pylint: disable=E1126 - dtype=np.int8, - ) -) # pylint: disable=E1126 -TEMPLATE["required"][ - "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/posint_value/@units" -] = "kg" # pylint: disable=E1126 -TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/char_value"] = ( - "just chars" # pylint: disable=E1126 -) -TEMPLATE["required"][ - "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/char_value/@units" -] = "" -TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/type"] = "2nd type" # pylint: disable=E1126 -TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/type/@array"] = [ - 0, - 1, - 2, -] -TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/date_value"] = ( - "2022-01-22T12:14:12.05018+00:00" # pylint: disable=E1126 -) -TEMPLATE["required"][ - "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/date_value/@units" -] = "" -TEMPLATE["required"]["/ENTRY[my_entry]/OPTIONAL_group[my_group]/required_field"] = 1 # pylint: disable=E1126 -TEMPLATE["required"]["/ENTRY[my_entry]/definition"] = "NXtest" # pylint: disable=E1126 -TEMPLATE["required"]["/ENTRY[my_entry]/definition/@version"] = "2.4.6" # pylint: disable=E1126 -TEMPLATE["required"]["/ENTRY[my_entry]/program_name"] = "Testing program" # pylint: disable=E1126 -TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/type"] = "2nd type" # pylint: disable=E1126 -TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/type/@array"] = [0, 1, 2] -TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/date_value"] = ( - "2022-01-22T12:14:12.05018+00:00" # pylint: disable=E1126 -) -TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/date_value/@units"] = "" -TEMPLATE["optional"]["/ENTRY[my_entry]/OPTIONAL_group[my_group]/optional_field"] = 1 -TEMPLATE["optional"]["/ENTRY[my_entry]/required_group/description"] = ( - "An example description" -) -TEMPLATE["optional"]["/ENTRY[my_entry]/required_group2/description"] = ( - "An example description" -) -TEMPLATE["required"][ - "/ENTRY[my_entry]/optional_parent/req_group_in_opt_group/DATA[data]" -] = 1 -TEMPLATE["lone_groups"] = [ - "/ENTRY[entry]/required_group", - "/ENTRY[entry]/required_group2", - "/ENTRY[entry]/optional_parent/req_group_in_opt_group", -] -TEMPLATE["optional"]["/@default"] = "Some NXroot attribute" - -# "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/in" -# "t_value should be one of: (, , )," -# " as defined in the NXDL as NX_INT." - - -# pylint: disable=too-many-arguments @pytest.mark.parametrize( - "data_dict,error_message", + "input_data, expected_output", [ - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/anamethatRENAMES[anamethatichangetothis]", - "not_a_num", - ), - ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/anamethatRENAMES[anamethatichangetothis]" - " should be one of the following Python types: (, ), as defined in " - "the NXDL as NX_INT." - ), - id="variadic-field-str-instead-of-int", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value", - "not_a_num", - ), - ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/in" - "t_value should be one of the following Python types: (, ), as defined in " - "the NXDL as NX_INT." - ), - id="string-instead-of-int", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value", - "NOT_TRUE_OR_FALSE", - ), - ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value should be one of the following Python types: (, ), as defined in the NXDL as NX_BOOLEAN." - ), - id="string-instead-of-bool", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value", - ["1", "2", "3"], - ), - ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value should" - " be one of the following Python types: (, ), as defined in the NXDL as NX_INT." - ), - id="list-of-int-str-instead-of-int", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value", - np.array([2.0, 3.0, 4.0], dtype=np.float32), - ), - ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value should be" - " one of the following Python types: (, ), as defined in the NXDL as NX_INT." - ), - id="array-of-float-instead-of-int", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value", - [2, 3, 4], - ), - (""), - id="list-of-int-instead-of-int", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value", - np.array([2, 3, 4], dtype=np.int32), - ), - (""), - id="array-of-int32-instead-of-int", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/date_value", - "2022-01-22T12:14:12.05018-00:00", - ), - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/date_value" - " = 2022-01-22T12:14:12.05018-00:00 should be a timezone aware" - " ISO8601 formatted str. For example, 2022-01-22T12:14:12.05018Z or 2022-01-22" - "T12:14:12.05018+00:00.", - id="int-instead-of-date", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value", - 0, - ), - ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value should be one of the following Python types: (, ), as defined in the NXDL as NX_FLOAT." - ), - id="int-instead-of-float", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/number_value", - "0", - ), - ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/number_value should be one of the following Python types: (, , , ), as defined in the NXDL as NX_NUMBER." - ), - id="str-instead-of-number", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", - np.array([0.0, 2]), - ), - ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value should be one" - " of the following Python types: (, ), as" - " defined in the NXDL as NX_CHAR." - ), - id="wrong-type-ndarray-instead-of-char", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", - np.array(["x", "2"]), - ), - (""), - id="valid-ndarray-instead-of-char", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value", - {"link": "/a-link"}, - ), - (""), - id="link-dict-instead-of-int", - ), - pytest.param( - alter_dict( - TEMPLATE, "/ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value", -1 - ), - ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value " - "should be a positive int, but is -1." - ), - id="negative-posint", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value", - [-1, 2], - ), - ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value " - "should be a positive int, but is [-1, 2]." - ), - id="negative-posint-list", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value", - np.array([-1, 2], dtype=np.int8), - ), - ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value should" - " be a positive int, but is [-1 2]." - ), - id="negative-posint-array", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value", - [1, 2], - ), - (""), - id="positive-posint-list", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value", - np.array([1, 2], dtype=np.int8), - ), - (""), - id="positive-posint-array", - ), - pytest.param( - alter_dict( - TEMPLATE, "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", 3 - ), - ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value should be one of the following Python types:" - " (, )," - " as defined in the NXDL as NX_CHAR." - ), - id="int-instead-of-chars", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", - np.array(["1", "2", "3"], dtype=np.str_), - ), - (""), - id="array-of-chars", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", - np.array(["1", "2", "3"], dtype=np.bytes_), - ), - (""), - id="array-of-bytes-chars", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", - ["list", "of", "chars"], - ), - "", - id="list-of-string-instead-of-chars", - ), - pytest.param( - alter_dict( - TEMPLATE, "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value", None - ), - "", - id="empty-optional-field", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value", - np.array([2.0, 3.0, 4.0], dtype=np.float32), - ), - "", - id="array-of-float-instead-of-float", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value", - np.array(["2.0", "3.0"], dtype=np.str_), - ), - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value should be " - "one of the following Python types: (, ), as defined in the NXDL " - "as NX_FLOAT.", - id="array-of-str-instead-of-float", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value", - [2], # pylint: disable=E1126 - ), - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value should be " - "one of the following Python types: (, ), as defined in the NXDL " - "as NX_FLOAT.", - id="list-of-int-instead-of-float", - ), - pytest.param( - set_to_none_in_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value", - "required", - ), - ( - "The data entry corresponding to /ENTRY[my_entry]/NXODD_name[nxodd_name]" - "/bool_value is" - " required and hasn't been supplied by the reader." - ), - id="empty-required-field", - ), - pytest.param( - set_to_none_in_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/bool_value", - "required", - ), - ( - "The data entry corresponding to /ENTRY[my_entry]/" - "NXODD_name[nxodd_two_name]/bool_value is" - " required and hasn't been supplied by the reader." - ), - id="empty-required-field", - ), - pytest.param( - remove_from_dict( - remove_from_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/bool_value", - "required", - ), - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value", - "required", - ), - ( - "The data entry corresponding to /ENTRY[my_entry]/NXODD_name[nxodd_name]" - "/bool_value is" - " required and hasn't been supplied by the reader." - ), - id="empty-required-field", - ), - pytest.param( - set_whole_group_to_none( - set_whole_group_to_none( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name", - "required", - ), - "/ENTRY[my_entry]/NXODD_name", - "optional", - ), - ("The required group, /ENTRY[my_entry]/NXODD_name, hasn't been supplied."), - id="all-required-fields-set-to-none", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/date_value", - "2022-01-22T12:14:12.05018+00:00", - ), - "", - id="UTC-with-+00:00", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/date_value", - "2022-01-22T12:14:12.05018Z", - ), - "", - id="UTC-with-Z", - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/date_value", - "2022-01-22T12:14:12.05018-00:00", - ), - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/date_value" - " = 2022-01-22T12:14:12.05018-00:00 should be a timezone aware" - " ISO8601 formatted str. For example, 2022-01-22T12:14:12.05018Z or 2022-01-22" - "T12:14:12.05018+00:00.", - id="UTC-with--00:00", - ), - pytest.param(listify_template(TEMPLATE), "", id="lists"), - pytest.param( - alter_dict( - TEMPLATE, "/ENTRY[my_entry]/NXODD_name[nxodd_name]/type", "Wrong option" - ), - ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/type should " - "be one of the following" - ": ['1st type', '2nd type', '3rd type', '4th type']" - ), - id="wrong-enum-choice", - ), - pytest.param( - set_to_none_in_dict( - TEMPLATE, "/ENTRY[my_entry]/optional_parent/required_child", "optional" - ), - ( - "The data entry corresponding to /ENTRY[my_entry]/optional_parent/" - "required_child is required and hasn't been supplied by the reader." - ), - id="atleast-one-required-child-not-provided-optional-parent", - ), - pytest.param( - set_to_none_in_dict( - TEMPLATE, - "/ENTRY[my_entry]/OPTIONAL_group[my_group]/required_field", - "required", - ), - ( - "The data entry corresponding to /ENTRY[my_entry]/" - "OPTIONAL_group[my_group]/required_field " - "is required and hasn't been supplied by the reader." - ), - id="required-field-not-provided-in-variadic-optional-group", - ), - pytest.param( - set_to_none_in_dict( - TEMPLATE, - "/ENTRY[my_entry]/OPTIONAL_group[my_group]/optional_field", - "required", - ), - (""), - id="required-field-provided-in-variadic-optional-group", - ), - pytest.param( - alter_dict( - alter_dict( - TEMPLATE, "/ENTRY[my_entry]/optional_parent/required_child", None - ), - "/ENTRY[my_entry]/optional_parent/optional_child", - None, - ), - (""), - id="no-child-provided-optional-parent", - ), - pytest.param(TEMPLATE, "", id="valid-data-dict"), - pytest.param( - remove_from_dict(TEMPLATE, "/ENTRY[my_entry]/required_group/description"), - "The required group, /ENTRY[my_entry]/required_group, hasn't been supplied.", - id="missing-empty-yet-required-group", - ), - pytest.param( - remove_from_dict(TEMPLATE, "/ENTRY[my_entry]/required_group2/description"), - "The required group, /ENTRY[my_entry]/required_group2, hasn't been supplied.", - id="missing-empty-yet-required-group2", - ), - pytest.param( - alter_dict( - remove_from_dict( - TEMPLATE, "/ENTRY[my_entry]/required_group/description" - ), - "/ENTRY[entry]/required_group", - None, - ), - "The required group, /ENTRY[my_entry]/required_group, hasn't been supplied.", - id="allow-required-and-empty-group", - ), - pytest.param( - remove_from_dict( - TEMPLATE, - "/ENTRY[my_entry]/optional_parent/req_group_in_opt_group/DATA[data]", - "required", - ), - ( - "The required group, /ENTRY[my_entry]/" - "optional_parent/req_group_in_opt_group, " - "hasn't been supplied." - ), - id="req-group-in-opt-parent-removed", - ), - pytest.param( - remove_optional_parent(TEMPLATE), (""), id="opt-group-completely-removed" - ), - pytest.param( - alter_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/type/@array", - ["0", 1, 2], - ), - ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/type/@array should be one of the following: [[0, 1, 2], [2, 3, 4]]" - ), - id="wrong-type-array-in-attribute", - ), - pytest.param( - alter_dict( - TEMPLATE, "/ENTRY[my_entry]/NXODD_name[nxodd_name]/type/@array", [1, 2] - ), - ( - "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/type/@array should be one of the following: [[0, 1, 2], [2, 3, 4]]" - ), - id="wrong-value-array-in-attribute", - ), + ("2.4E-23", 2.4e-23), + ("28", 28), + ("45.98", 45.98), + ("test", "test"), + (["59", "3.00005", "498E-36"], np.array([59.0, 3.00005, 4.98e-34])), + ("23 34 444 5000", np.array([23.0, 34.0, 444.0, 5000.0])), + ("xrd experiment", "xrd experiment"), + (None, None), ], ) -def test_validate_data_dict(caplog, data_dict, error_message, request): - """Unit test for the data validation routine.""" - - def format_error_message(msg: str) -> str: - return msg[msg.rfind("G: ") + 3 :].rstrip("\n") - - if request.node.callspec.id in ( - "valid-data-dict", - "lists", - "empty-optional-field", - "UTC-with-+00:00", - "UTC-with-Z", - "no-child-provided-optional-parent", - "link-dict-instead-of-int", - "opt-group-completely-removed", - "required-field-provided-in-variadic-optional-group", - "valid-ndarray-instead-of-char", - "list-of-int-instead-of-int", - "list-of-string-instead-of-chars", - "array-of-int32-instead-of-int", - "List-of-int-instead-of-int", - "positive-posint-list", - "positive-posint-array", - "array-of-chars", - "array-of-bytes-chars", - "array-of-float-instead-of-float", - "numpy-chararray", - ): - with caplog.at_level(logging.WARNING): - assert validate_dict_against("NXtest", data_dict)[0] - assert caplog.text == "" - # Missing required fields caught by logger with warning - elif request.node.callspec.id in ( - "empty-required-field", - "allow-required-and-empty-group", - "req-group-in-opt-parent-removed", - "missing-empty-yet-required-group", - "missing-empty-yet-required-group2", - ): - assert "" == caplog.text - captured_logs = caplog.records - assert not validate_dict_against("NXtest", data_dict)[0] - assert any( - error_message == format_error_message(rec.message) for rec in captured_logs - ) +def test_transform_to_intended_dt(input_data, expected_output): + """Transform to possible numerical method.""" + result = helpers.transform_to_intended_dt(input_data) + + # Use pytest.approx for comparing floating-point numbers + if isinstance(expected_output, np.ndarray): + np.testing.assert_allclose(result, expected_output, rtol=1e-3) + elif isinstance(expected_output, float): + assert result == pytest.approx(expected_output, rel=1e-5) else: - with caplog.at_level(logging.WARNING): - assert not validate_dict_against("NXtest", data_dict)[0] - assert any( - error_message == format_error_message(rec.message) for rec in caplog.records - ) + assert result == expected_output @pytest.mark.parametrize( diff --git a/tests/dataconverter/test_validation.py b/tests/dataconverter/test_validation.py index 8bb892998..3d159d821 100644 --- a/tests/dataconverter/test_validation.py +++ b/tests/dataconverter/test_validation.py @@ -17,12 +17,82 @@ # limitations under the License. # import logging -from typing import Any, Dict, List, Tuple, Union +from typing import Optional import numpy as np import pytest +from pynxtools.dataconverter.template import Template from pynxtools.dataconverter.validation import validate_dict_against +from .test_helpers import ( # pylint: disable=unused-import + alter_dict, + fixture_filled_test_data, + fixture_template, +) + + +def remove_optional_parent(data_dict: Template): + """Completely removes the optional group from the test Template.""" + internal_dict = Template(data_dict) + del internal_dict["/ENTRY[my_entry]/optional_parent/required_child"] + del internal_dict["/ENTRY[my_entry]/optional_parent/optional_child"] + del internal_dict[ + "/ENTRY[my_entry]/optional_parent/req_group_in_opt_group/DATA[data]" + ] + + return internal_dict + + +def set_to_none_in_dict(data_dict: Optional[Template], key: str, optionality: str): + """Helper function to forcefully set path to 'None'""" + if data_dict is None: + return None + + internal_dict = Template(data_dict) + internal_dict[optionality][key] = None + return internal_dict + + +def set_whole_group_to_none( + data_dict: Optional[Template], key: str, optionality: str +) -> Optional[Template]: + """Set a whole path to None in the dict""" + if data_dict is None: + return None + + internal_dict = Template(data_dict) + for path in data_dict[optionality]: + if path.startswith(key): + internal_dict[optionality][path] = None + return internal_dict + + +def remove_from_dict(data_dict: Template, key: str, optionality: str = "optional"): + """Helper function to remove a key from dict""" + if data_dict is not None and key in data_dict[optionality]: + internal_dict = Template(data_dict) + del internal_dict[optionality][key] + return internal_dict + + return None + + +def listify_template(data_dict: Template): + """Helper function to turn most values in the Template into lists""" + listified_template = Template() + for optionality in ("optional", "recommended", "required", "undocumented"): + for path in data_dict[optionality]: + if path[path.rindex("/") + 1 :] in ( + "@units", + "type", + "definition", + "date_value", + ) or isinstance(data_dict[optionality][path], list): + listified_template[optionality][path] = data_dict[optionality][path] + else: + listified_template[optionality][path] = [data_dict[optionality][path]] + return listified_template + def get_data_dict(): return { @@ -73,20 +143,627 @@ def get_data_dict(): } -def remove_from_dict(keys: Union[Union[List[str], Tuple[str, ...]], str], data_dict): - if isinstance(keys, (list, tuple)): - for key in keys: - data_dict.pop(key, None) - else: - data_dict.pop(keys) +TEMPLATE = Template() +TEMPLATE["optional"][ + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/anamethatRENAMES[anamethatichangetothis]" +] = 2 +TEMPLATE["optional"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value"] = 2.0 # pylint: disable=E1126 +TEMPLATE["optional"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value/@units"] = ( + "nm" # pylint: disable=E1126 +) +TEMPLATE["optional"]["/ENTRY[my_entry]/optional_parent/required_child"] = 1 # pylint: disable=E1126 +TEMPLATE["optional"]["/ENTRY[my_entry]/optional_parent/optional_child"] = 1 # pylint: disable=E1126 +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value"] = True # pylint: disable=E1126 +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value/@units"] = "" +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value"] = 2 # pylint: disable=E1126 +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value/@units"] = "eV" # pylint: disable=E1126 +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/number_value"] = 2 +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/number_value/@units"] = ( + "eV" +) +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value"] = np.array( + [1, 2, 3], # pylint: disable=E1126 + dtype=np.int8, +) # pylint: disable=E1126 +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value/@units"] = ( + "kg" # pylint: disable=E1126 +) +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value"] = ( + "just chars" # pylint: disable=E1126 +) +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value/@units"] = "" +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/bool_value"] = True # pylint: disable=E1126 +TEMPLATE["required"][ + "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/bool_value/@units" +] = "" +TEMPLATE["required"][ + "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/anamethatRENAMES[anamethatichangetothis]" +] = 2 # pylint: disable=E1126 +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/int_value"] = 2 # pylint: disable=E1126 +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/int_value/@units"] = ( + "eV" # pylint: disable=E1126 +) +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/posint_value"] = ( + np.array( + [1, 2, 3], # pylint: disable=E1126 + dtype=np.int8, + ) +) # pylint: disable=E1126 +TEMPLATE["required"][ + "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/posint_value/@units" +] = "kg" # pylint: disable=E1126 +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/char_value"] = ( + "just chars" # pylint: disable=E1126 +) +TEMPLATE["required"][ + "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/char_value/@units" +] = "" +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/type"] = "2nd type" # pylint: disable=E1126 +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/type/@array"] = [ + 0, + 1, + 2, +] +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/date_value"] = ( + "2022-01-22T12:14:12.05018+00:00" # pylint: disable=E1126 +) +TEMPLATE["required"][ + "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/date_value/@units" +] = "" +TEMPLATE["required"]["/ENTRY[my_entry]/OPTIONAL_group[my_group]/required_field"] = 1 # pylint: disable=E1126 +TEMPLATE["required"]["/ENTRY[my_entry]/definition"] = "NXtest" # pylint: disable=E1126 +TEMPLATE["required"]["/ENTRY[my_entry]/definition/@version"] = "2.4.6" # pylint: disable=E1126 +TEMPLATE["required"]["/ENTRY[my_entry]/program_name"] = "Testing program" # pylint: disable=E1126 +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/type"] = "2nd type" # pylint: disable=E1126 +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/type/@array"] = [0, 1, 2] +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/date_value"] = ( + "2022-01-22T12:14:12.05018+00:00" # pylint: disable=E1126 +) +TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/date_value/@units"] = "" +TEMPLATE["optional"]["/ENTRY[my_entry]/OPTIONAL_group[my_group]/optional_field"] = 1 +TEMPLATE["optional"]["/ENTRY[my_entry]/required_group/description"] = ( + "An example description" +) +TEMPLATE["optional"]["/ENTRY[my_entry]/required_group2/description"] = ( + "An example description" +) +TEMPLATE["required"][ + "/ENTRY[my_entry]/optional_parent/req_group_in_opt_group/DATA[data]" +] = 1 +TEMPLATE["lone_groups"] = [ + "/ENTRY[entry]/required_group", + "/ENTRY[entry]/required_group2", + "/ENTRY[entry]/optional_parent/req_group_in_opt_group", +] +TEMPLATE["optional"]["/@default"] = "Some NXroot attribute" - return data_dict +# pylint: disable=too-many-arguments +@pytest.mark.parametrize( + "data_dict,error_message", + [ + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/anamethatRENAMES[anamethatichangetothis]", + "not_a_num", + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/anamethatRENAMES[anamethatichangetothis]" + " should be one of the following Python types: (, ), as defined in " + "the NXDL as NX_INT." + ), + id="variadic-field-str-instead-of-int", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value", + "not_a_num", + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/in" + "t_value should be one of the following Python types: (, ), as defined in " + "the NXDL as NX_INT." + ), + id="string-instead-of-int", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value", + "NOT_TRUE_OR_FALSE", + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value should be one of the following Python types: (, ), as defined in the NXDL as NX_BOOLEAN." + ), + id="string-instead-of-bool", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value", + ["1", "2", "3"], + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value should" + " be one of the following Python types: (, ), as defined in the NXDL as NX_INT." + ), + id="list-of-int-str-instead-of-int", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value", + np.array([2.0, 3.0, 4.0], dtype=np.float32), + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value should be" + " one of the following Python types: (, ), as defined in the NXDL as NX_INT." + ), + id="array-of-float-instead-of-int", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value", + [2, 3, 4], + ), + (""), + id="list-of-int-instead-of-int", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value", + np.array([2, 3, 4], dtype=np.int32), + ), + (""), + id="array-of-int32-instead-of-int", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/date_value", + "2022-01-22T12:14:12.05018-00:00", + ), + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/date_value" + " = 2022-01-22T12:14:12.05018-00:00 should be a timezone aware" + " ISO8601 formatted str. For example, 2022-01-22T12:14:12.05018Z or 2022-01-22" + "T12:14:12.05018+00:00.", + id="int-instead-of-date", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value", + 0, + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value should be one of the following Python types: (, ), as defined in the NXDL as NX_FLOAT." + ), + id="int-instead-of-float", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/number_value", + "0", + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/number_value should be one of the following Python types: (, , , ), as defined in the NXDL as NX_NUMBER." + ), + id="str-instead-of-number", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", + np.array([0.0, 2]), + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value should be one" + " of the following Python types: (, ), as" + " defined in the NXDL as NX_CHAR." + ), + id="wrong-type-ndarray-instead-of-char", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", + np.array(["x", "2"]), + ), + (""), + id="valid-ndarray-instead-of-char", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value", + {"link": "/a-link"}, + ), + (""), + id="link-dict-instead-of-int", + ), + pytest.param( + alter_dict( + TEMPLATE, "/ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value", -1 + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value " + "should be a positive int, but is -1." + ), + id="negative-posint", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value", + [-1, 2], + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value " + "should be a positive int, but is [-1, 2]." + ), + id="negative-posint-list", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value", + np.array([-1, 2], dtype=np.int8), + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value should" + " be a positive int, but is [-1 2]." + ), + id="negative-posint-array", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value", + [1, 2], + ), + (""), + id="positive-posint-list", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value", + np.array([1, 2], dtype=np.int8), + ), + (""), + id="positive-posint-array", + ), + pytest.param( + alter_dict( + TEMPLATE, "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", 3 + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value should be one of the following Python types:" + " (, )," + " as defined in the NXDL as NX_CHAR." + ), + id="int-instead-of-chars", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", + np.array(["1", "2", "3"], dtype=np.str_), + ), + (""), + id="array-of-chars", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", + np.array(["1", "2", "3"], dtype=np.bytes_), + ), + (""), + id="array-of-bytes-chars", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value", + ["list", "of", "chars"], + ), + "", + id="list-of-string-instead-of-chars", + ), + pytest.param( + alter_dict( + TEMPLATE, "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value", None + ), + "", + id="empty-optional-field", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value", + np.array([2.0, 3.0, 4.0], dtype=np.float32), + ), + "", + id="array-of-float-instead-of-float", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value", + np.array(["2.0", "3.0"], dtype=np.str_), + ), + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value should be " + "one of the following Python types: (, ), as defined in the NXDL " + "as NX_FLOAT.", + id="array-of-str-instead-of-float", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value", + [2], # pylint: disable=E1126 + ), + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value should be " + "one of the following Python types: (, ), as defined in the NXDL " + "as NX_FLOAT.", + id="list-of-int-instead-of-float", + ), + pytest.param( + set_to_none_in_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value", + "required", + ), + ( + "The data entry corresponding to /ENTRY[my_entry]/NXODD_name[nxodd_name]" + "/bool_value is" + " required and hasn't been supplied by the reader." + ), + id="empty-required-field", + ), + pytest.param( + set_to_none_in_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/bool_value", + "required", + ), + ( + "The data entry corresponding to /ENTRY[my_entry]/" + "NXODD_name[nxodd_two_name]/bool_value is" + " required and hasn't been supplied by the reader." + ), + id="empty-required-field", + ), + pytest.param( + remove_from_dict( + remove_from_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/bool_value", + "required", + ), + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value", + "required", + ), + ( + "The data entry corresponding to /ENTRY[my_entry]/NXODD_name[nxodd_name]" + "/bool_value is" + " required and hasn't been supplied by the reader." + ), + id="empty-required-field", + ), + pytest.param( + set_whole_group_to_none( + set_whole_group_to_none( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name", + "required", + ), + "/ENTRY[my_entry]/NXODD_name", + "optional", + ), + ("The required group, /ENTRY[my_entry]/NXODD_name, hasn't been supplied."), + id="all-required-fields-set-to-none", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/date_value", + "2022-01-22T12:14:12.05018+00:00", + ), + "", + id="UTC-with-+00:00", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/date_value", + "2022-01-22T12:14:12.05018Z", + ), + "", + id="UTC-with-Z", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/date_value", + "2022-01-22T12:14:12.05018-00:00", + ), + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/date_value" + " = 2022-01-22T12:14:12.05018-00:00 should be a timezone aware" + " ISO8601 formatted str. For example, 2022-01-22T12:14:12.05018Z or 2022-01-22" + "T12:14:12.05018+00:00.", + id="UTC-with--00:00", + ), + pytest.param(listify_template(TEMPLATE), "", id="lists"), + pytest.param( + alter_dict( + TEMPLATE, "/ENTRY[my_entry]/NXODD_name[nxodd_name]/type", "Wrong option" + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/type should " + "be one of the following" + ": ['1st type', '2nd type', '3rd type', '4th type']" + ), + id="wrong-enum-choice", + ), + pytest.param( + set_to_none_in_dict( + TEMPLATE, "/ENTRY[my_entry]/optional_parent/required_child", "optional" + ), + ( + "The data entry corresponding to /ENTRY[my_entry]/optional_parent/" + "required_child is required and hasn't been supplied by the reader." + ), + id="atleast-one-required-child-not-provided-optional-parent", + ), + pytest.param( + set_to_none_in_dict( + TEMPLATE, + "/ENTRY[my_entry]/OPTIONAL_group[my_group]/required_field", + "required", + ), + ( + "The data entry corresponding to /ENTRY[my_entry]/" + "OPTIONAL_group[my_group]/required_field " + "is required and hasn't been supplied by the reader." + ), + id="required-field-not-provided-in-variadic-optional-group", + ), + pytest.param( + set_to_none_in_dict( + TEMPLATE, + "/ENTRY[my_entry]/OPTIONAL_group[my_group]/optional_field", + "required", + ), + (""), + id="required-field-provided-in-variadic-optional-group", + ), + pytest.param( + alter_dict( + alter_dict( + TEMPLATE, "/ENTRY[my_entry]/optional_parent/required_child", None + ), + "/ENTRY[my_entry]/optional_parent/optional_child", + None, + ), + (""), + id="no-child-provided-optional-parent", + ), + pytest.param(TEMPLATE, "", id="valid-data-dict"), + pytest.param( + remove_from_dict(TEMPLATE, "/ENTRY[my_entry]/required_group/description"), + "The required group, /ENTRY[my_entry]/required_group, hasn't been supplied.", + id="missing-empty-yet-required-group", + ), + pytest.param( + remove_from_dict(TEMPLATE, "/ENTRY[my_entry]/required_group2/description"), + "The required group, /ENTRY[my_entry]/required_group2, hasn't been supplied.", + id="missing-empty-yet-required-group2", + ), + pytest.param( + alter_dict( + remove_from_dict( + TEMPLATE, "/ENTRY[my_entry]/required_group/description" + ), + "/ENTRY[entry]/required_group", + None, + ), + "The required group, /ENTRY[my_entry]/required_group, hasn't been supplied.", + id="allow-required-and-empty-group", + ), + pytest.param( + remove_from_dict( + TEMPLATE, + "/ENTRY[my_entry]/optional_parent/req_group_in_opt_group/DATA[data]", + "required", + ), + ( + "The required group, /ENTRY[my_entry]/" + "optional_parent/req_group_in_opt_group, " + "hasn't been supplied." + ), + id="req-group-in-opt-parent-removed", + ), + pytest.param((TEMPLATE), (""), id="opt-group-completely-removed"), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/type/@array", + ["0", 1, 2], + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/type/@array should be one of the following: [[0, 1, 2], [2, 3, 4]]" + ), + id="wrong-type-array-in-attribute", + ), + pytest.param( + alter_dict( + TEMPLATE, "/ENTRY[my_entry]/NXODD_name[nxodd_name]/type/@array", [1, 2] + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name[nxodd_name]/type/@array should be one of the following: [[0, 1, 2], [2, 3, 4]]" + ), + id="wrong-value-array-in-attribute", + ), + ], +) +def test_validate_data_dict(caplog, data_dict, error_message, request): + """Unit test for the data validation routine.""" -def alter_dict(new_values: Dict[str, Any], data_dict: Dict[str, Any]) -> Dict[str, Any]: - for key, value in new_values.items(): - data_dict[key] = value - return data_dict + def format_error_message(msg: str) -> str: + return msg[msg.rfind("G: ") + 3 :].rstrip("\n") + + if request.node.callspec.id in ( + "valid-data-dict", + "lists", + "empty-optional-field", + "UTC-with-+00:00", + "UTC-with-Z", + "no-child-provided-optional-parent", + "link-dict-instead-of-int", + "opt-group-completely-removed", + "required-field-provided-in-variadic-optional-group", + "valid-ndarray-instead-of-char", + "list-of-int-instead-of-int", + "list-of-string-instead-of-chars", + "array-of-int32-instead-of-int", + "List-of-int-instead-of-int", + "positive-posint-list", + "positive-posint-array", + "array-of-chars", + "array-of-bytes-chars", + "array-of-float-instead-of-float", + "numpy-chararray", + ): + with caplog.at_level(logging.WARNING): + assert validate_dict_against("NXtest", data_dict)[0] + assert caplog.text == "" + # Missing required fields caught by logger with warning + elif request.node.callspec.id in ( + "empty-required-field", + "allow-required-and-empty-group", + "req-group-in-opt-parent-removed", + "missing-empty-yet-required-group", + "missing-empty-yet-required-group2", + ): + assert "" == caplog.text + captured_logs = caplog.records + assert not validate_dict_against("NXtest", data_dict)[0] + assert any( + error_message == format_error_message(rec.message) for rec in captured_logs + ) + else: + with caplog.at_level(logging.WARNING): + assert not validate_dict_against("NXtest", data_dict)[0] + assert any( + error_message == format_error_message(rec.message) for rec in caplog.records + ) @pytest.mark.parametrize( @@ -95,8 +772,9 @@ def alter_dict(new_values: Dict[str, Any], data_dict: Dict[str, Any]) -> Dict[st pytest.param(get_data_dict(), id="valid-unaltered-data-dict"), pytest.param( remove_from_dict( + TEMPLATE, "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value_no_attr", - get_data_dict(), + "optional", ), id="removed-optional-value", ), @@ -113,7 +791,9 @@ def test_valid_data_dict(caplog, data_dict): [ pytest.param( remove_from_dict( - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value", get_data_dict() + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value", + "optional", ), "The attribute /ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value/@units will not be written.", "There were attributes set for the field /ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value, but the field does not exist.", @@ -135,7 +815,9 @@ def test_data_dict_attr_with_no_field( [ pytest.param( remove_from_dict( - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value", get_data_dict() + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value", + "required", ), "The data entry corresponding to /ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value is required and hasn't been supplied by the reader.", id="missing-required-value", From ea22d42e8d2f654d27426502b47c459fe708f7c7 Mon Sep 17 00:00:00 2001 From: rettigl Date: Tue, 11 Mar 2025 20:44:56 +0100 Subject: [PATCH 33/36] fix converted tests --- tests/dataconverter/test_validation.py | 117 +++++++++---------------- 1 file changed, 40 insertions(+), 77 deletions(-) diff --git a/tests/dataconverter/test_validation.py b/tests/dataconverter/test_validation.py index 3d159d821..47e1956aa 100644 --- a/tests/dataconverter/test_validation.py +++ b/tests/dataconverter/test_validation.py @@ -31,18 +31,6 @@ ) -def remove_optional_parent(data_dict: Template): - """Completely removes the optional group from the test Template.""" - internal_dict = Template(data_dict) - del internal_dict["/ENTRY[my_entry]/optional_parent/required_child"] - del internal_dict["/ENTRY[my_entry]/optional_parent/optional_child"] - del internal_dict[ - "/ENTRY[my_entry]/optional_parent/req_group_in_opt_group/DATA[data]" - ] - - return internal_dict - - def set_to_none_in_dict(data_dict: Optional[Template], key: str, optionality: str): """Helper function to forcefully set path to 'None'""" if data_dict is None: @@ -151,6 +139,9 @@ def get_data_dict(): TEMPLATE["optional"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value/@units"] = ( "nm" # pylint: disable=E1126 ) +TEMPLATE["optional"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value_no_attr"] = ( + 2.0, +) TEMPLATE["optional"]["/ENTRY[my_entry]/optional_parent/required_child"] = 1 # pylint: disable=E1126 TEMPLATE["optional"]["/ENTRY[my_entry]/optional_parent/optional_child"] = 1 # pylint: disable=E1126 TEMPLATE["required"]["/ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value"] = True # pylint: disable=E1126 @@ -557,6 +548,42 @@ def get_data_dict(): ), id="empty-required-field", ), + pytest.param( + remove_from_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value_no_attr", + "optional", + ), + "", + id="removed-optional-value", + ), + pytest.param( + remove_from_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value", + "optional", + ), + "There were attributes set for the field /ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value, but the field does not exist.", + id="removed-optional-value-with-attribute-remaining", + ), + pytest.param( + remove_from_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value", + "optional", + ), + "The attribute /ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value/@units will not be written.", + id="removed-optional-value-with-attribute-remaining", + ), + pytest.param( + remove_from_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value", + "required", + ), + "The data entry corresponding to /ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value is required and hasn't been supplied by the reader.", + id="missing-required-value", + ), pytest.param( set_whole_group_to_none( set_whole_group_to_none( @@ -740,6 +767,7 @@ def format_error_message(msg: str) -> str: "array-of-bytes-chars", "array-of-float-instead-of-float", "numpy-chararray", + "removed-optional-value", ): with caplog.at_level(logging.WARNING): assert validate_dict_against("NXtest", data_dict)[0] @@ -764,68 +792,3 @@ def format_error_message(msg: str) -> str: assert any( error_message == format_error_message(rec.message) for rec in caplog.records ) - - -@pytest.mark.parametrize( - "data_dict", - [ - pytest.param(get_data_dict(), id="valid-unaltered-data-dict"), - pytest.param( - remove_from_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value_no_attr", - "optional", - ), - id="removed-optional-value", - ), - ], -) -def test_valid_data_dict(caplog, data_dict): - with caplog.at_level(logging.WARNING): - assert validate_dict_against("NXtest", data_dict)[0] - assert caplog.text == "" - - -@pytest.mark.parametrize( - "data_dict, error_message_1, error_message_2", - [ - pytest.param( - remove_from_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value", - "optional", - ), - "The attribute /ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value/@units will not be written.", - "There were attributes set for the field /ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value, but the field does not exist.", - id="removed-optional-value-with-attribute-remaining", - ), - ], -) -def test_data_dict_attr_with_no_field( - caplog, data_dict, error_message_1, error_message_2 -): - with caplog.at_level(logging.WARNING): - assert not validate_dict_against("NXtest", data_dict)[0] - assert error_message_1 in caplog.text - assert error_message_2 in caplog.text - - -@pytest.mark.parametrize( - "data_dict, error_message", - [ - pytest.param( - remove_from_dict( - TEMPLATE, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value", - "required", - ), - "The data entry corresponding to /ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value is required and hasn't been supplied by the reader.", - id="missing-required-value", - ) - ], -) -def test_validation_shows_warning(caplog, data_dict, error_message): - with caplog.at_level(logging.WARNING): - assert not validate_dict_against("NXtest", data_dict)[0] - - assert error_message in caplog.text From 8ffd63bc21e06f8cfc3c64770574323b32153f6e Mon Sep 17 00:00:00 2001 From: rettigl Date: Tue, 11 Mar 2025 21:39:09 +0100 Subject: [PATCH 34/36] add additional tests for base class elements --- tests/dataconverter/test_validation.py | 134 ++++++++++++++++--------- 1 file changed, 85 insertions(+), 49 deletions(-) diff --git a/tests/dataconverter/test_validation.py b/tests/dataconverter/test_validation.py index 47e1956aa..052771503 100644 --- a/tests/dataconverter/test_validation.py +++ b/tests/dataconverter/test_validation.py @@ -82,55 +82,6 @@ def listify_template(data_dict: Template): return listified_template -def get_data_dict(): - return { - "/ENTRY[my_entry]/optional_parent/required_child": 1, - "/ENTRY[my_entry]/optional_parent/optional_child": 1, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/anamethatRENAMES[anamethatichangetothis]": 2, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value_no_attr": 2.0, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value": 2.0, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value/@units": "nm", - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value": True, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/bool_value/@units": "", - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value": 2, - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/int_value/@units": "eV", - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value": np.array( - [1, 2, 3], dtype=np.int8 - ), - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/posint_value/@units": "kg", - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value": "just chars", - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/char_value/@units": "", - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/type": "2nd type", - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/type/@array": [0, 1, 2], - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/date_value": "2022-01-22T12:14:12.05018+00:00", - "/ENTRY[my_entry]/NXODD_name[nxodd_name]/date_value/@units": "", - "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/anamethatRENAMES[anamethatichangetothis]": 2, - "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/bool_value": True, - "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/bool_value/@units": "", - "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/int_value": 2, - "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/int_value/@units": "eV", - "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/posint_value": np.array( - [1, 2, 3], dtype=np.int8 - ), - "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/posint_value/@units": "kg", - "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/char_value": "just chars", - "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/char_value/@units": "", - "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/type": "2nd type", - "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/type/@array": [0, 1, 2], - "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/date_value": "2022-01-22T12:14:12.05018+00:00", - "/ENTRY[my_entry]/NXODD_name[nxodd_two_name]/date_value/@units": "", - "/ENTRY[my_entry]/OPTIONAL_group[my_group]/required_field": 1, - "/ENTRY[my_entry]/definition": "NXtest", - "/ENTRY[my_entry]/definition/@version": "2.4.6", - "/ENTRY[my_entry]/program_name": "Testing program", - "/ENTRY[my_entry]/OPTIONAL_group[my_group]/optional_field": 1, - "/ENTRY[my_entry]/required_group/description": "An example description", - "/ENTRY[my_entry]/required_group2/description": "An example description", - "/ENTRY[my_entry]/optional_parent/req_group_in_opt_group/data": 1, - "/@default": "Some NXroot attribute", - } - - TEMPLATE = Template() TEMPLATE["optional"][ "/ENTRY[my_entry]/NXODD_name[nxodd_name]/anamethatRENAMES[anamethatichangetothis]" @@ -227,6 +178,12 @@ def get_data_dict(): "/ENTRY[entry]/optional_parent/req_group_in_opt_group", ] TEMPLATE["optional"]["/@default"] = "Some NXroot attribute" +# keys not registered in appdef +TEMPLATE["required"]["/ENTRY[my_entry]/duration"] = 1 # pylint: disable=E1126 +TEMPLATE["required"]["/ENTRY[my_entry]/duration/@units"] = "s" # pylint: disable=E1126 +TEMPLATE["required"][ + "/ENTRY[my_entry]/INSTRUMENT[my_instrument]/SOURCE[my_source]/type" +] = "Ion Source" # pylint: disable=E1126 # pylint: disable=too-many-arguments @@ -738,6 +695,85 @@ def get_data_dict(): ), id="wrong-value-array-in-attribute", ), + pytest.param( + remove_from_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/number_value/@units", + "required", + ), + "Field /ENTRY[my_entry]/NXODD_name[nxodd_name]/number_value requires a unit in the unit category NX_ENERGY.", + id="missing-unit", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/duration", + np.array([2.0, 3.0, 4.0], dtype=np.float32), + ), + ( + "The value at /ENTRY[my_entry]/duration should be" + " one of the following Python types: (, ), as defined in the NXDL as NX_INT." + ), + id="baseclass-wrong-dtype", + ), + pytest.param( + remove_from_dict( + TEMPLATE, + "/ENTRY[my_entry]/duration/@units", + "required", + ), + "Field /ENTRY[my_entry]/duration requires a unit in the unit category NX_TIME.", + id="baseclass-missing-unit", + ), + pytest.param( + remove_from_dict( + TEMPLATE, + "/ENTRY[my_entry]/duration", + "required", + ), + ( + "There were attributes set for the field /ENTRY[my_entry]/duration, but the field does not exist." + ), + id="baseclass-attribute-missing-field", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/INSTRUMENT[my_instrument]/SOURCE[my_source]/type", + "Wrong source type", + ), + ( + "The value at /ENTRY[my_entry]/INSTRUMENT[my_instrument]/SOURCE[my_source]/type " + "should be one of the following: ['Spallation Neutron Source', 'Pulsed Reactor Neutron Source', " + "'Reactor Neutron Source', 'Synchrotron X-ray Source', 'Pulsed Muon Source', 'Rotating Anode X-ray', " + "'Fixed Tube X-ray', 'UV Laser', 'Free-Electron Laser', 'Optical Laser', 'Ion Source', 'UV Plasma Source', " + "'Metal Jet X-ray', 'Laser', 'Dye-Laser', 'Broadband Tunable Light Source', 'Halogen lamp', 'LED', " + "'Mercury Cadmium Telluride', 'Deuterium Lamp', 'Xenon Lamp', 'Globar', 'other']" + ), + id="baseclass-wrong-enum", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/INSTRUMENT[my_instrument]/SOURCE[my_source]/illegal_name", + 1, + ), + ( + "Field /ENTRY[my_entry]/INSTRUMENT[my_instrument]/SOURCE[my_source]/illegal_name written without documentation." + ), + id="add-undocumented-field", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/INSTRUMENT[my_instrument]/SOURCE[my_source]/type/@illegal", + "illegal_attribute", + ), + ( + "Attribute /ENTRY[my_entry]/INSTRUMENT[my_instrument]/SOURCE[my_source]/type/illegal written without documentation." + ), + id="add-undocumented-attribute", + ), ], ) def test_validate_data_dict(caplog, data_dict, error_message, request): From 02f272132d8aa3c161382857ab0ae1f4c4cca562 Mon Sep 17 00:00:00 2001 From: rettigl Date: Wed, 12 Mar 2025 22:27:50 +0100 Subject: [PATCH 35/36] fix validation issues and add further tests --- src/pynxtools/dataconverter/helpers.py | 7 +- src/pynxtools/dataconverter/validation.py | 59 ++++++++--- tests/dataconverter/test_validation.py | 115 ++++++++++++++++++++-- 3 files changed, 157 insertions(+), 24 deletions(-) diff --git a/src/pynxtools/dataconverter/helpers.py b/src/pynxtools/dataconverter/helpers.py index 9813e8898..e6851033d 100644 --- a/src/pynxtools/dataconverter/helpers.py +++ b/src/pynxtools/dataconverter/helpers.py @@ -114,7 +114,10 @@ def _log(self, path: str, log_type: ValidationProblem, value: Optional[Any], *ar f"Expected a group at {path} but found a field or attribute." ) elif log_type == ValidationProblem.MissingDocumentation: - logger.warning(f"Field {path} written without documentation.") + if "@" in path.rsplit("/")[-1]: + logger.warning(f"Attribute {path} written without documentation.") + else: + logger.warning(f"Field {path} written without documentation.") elif log_type == ValidationProblem.MissingUnit: logger.warning( f"Field {path} requires a unit in the unit category {value}." @@ -122,7 +125,7 @@ def _log(self, path: str, log_type: ValidationProblem, value: Optional[Any], *ar elif log_type == ValidationProblem.MissingRequiredAttribute: logger.warning(f'Missing attribute: "{path}"') elif log_type == ValidationProblem.UnitWithoutField: - logger.warning(f"Unit {path} in dataset without its field {value}") + logger.warning(f"Unit {path} in dataset without its field {value}.") elif log_type == ValidationProblem.AttributeForNonExistingField: logger.warning( f"There were attributes set for the field {path}, " diff --git a/src/pynxtools/dataconverter/validation.py b/src/pynxtools/dataconverter/validation.py index ae244e701..543eb3ce6 100644 --- a/src/pynxtools/dataconverter/validation.py +++ b/src/pynxtools/dataconverter/validation.py @@ -508,19 +508,26 @@ def handle_unknown_type(node: NexusNode, keys: Mapping[str, Any], prev_path: str # TODO: Raise error or log the issue? pass - def is_documented(key: str, node: NexusNode) -> bool: - if mapping.get(key) is None: - # This value is not really set. Skip checking it's documentation. - return True - + def add_best_matches_for(key: str, node: NexusNode) -> Optional[NexusNode]: for name in key[1:].replace("@", "").split("/"): children = node.get_all_direct_children_names() best_name = best_namefit_of(name, children) if best_name is None: - return False + return None node = node.search_add_child_for(best_name) + return node + + def is_documented(key: str, node: NexusNode) -> bool: + if mapping.get(key) is None: + # This value is not really set. Skip checking it's documentation. + return True + + node = add_best_matches_for(key, node) + if node is None: + return False + if isinstance(mapping[key], dict) and "link" in mapping[key]: # TODO: Follow link and check consistency with current field return True @@ -584,9 +591,10 @@ def check_attributes_of_nonexisting_field( for key in mapping: last_index = key.rfind("/") - if key[last_index + 1] == "@": + if key[last_index + 1] == "@" and key[last_index + 1 :] != "@units": # key is an attribute. Find a corresponding parent, check all the other # children of this parent + # ignore units here, they are checked separately attribute_parent_checked = False for key_iterating in mapping: # check if key_iterating starts with parent of the key OR any @@ -756,22 +764,46 @@ def startswith_with_variations( not_visited = list(mapping) recurse_tree(tree, nested_keys) + keys_to_remove = check_attributes_of_nonexisting_field(tree) + for not_visited_key in not_visited: if not_visited_key.endswith("/@units"): - if is_documented(not_visited_key.rsplit("/", 1)[0], tree): - continue - if not_visited_key.rsplit("/", 1)[0] not in not_visited: + # check that parent exists + if not_visited_key.rsplit("/", 1)[0] not in mapping.keys(): collector.collect_and_log( not_visited_key, ValidationProblem.UnitWithoutField, not_visited_key.rsplit("/", 1)[0], ) - if not ignore_undocumented: collector.collect_and_log( not_visited_key, - ValidationProblem.UnitWithoutDocumentation, - mapping[not_visited_key], + ValidationProblem.KeyToBeRemoved, + None, ) + keys_to_remove.append(not_visited_key) + + # parent key will be checked on its own if it exists, because it is in the list + continue + + if "@" in not_visited_key.rsplit("/")[-1]: + # check that parent exists + if not_visited_key.rsplit("/", 1)[0] not in mapping.keys(): + # check that parent is not a group + node = add_best_matches_for(not_visited_key.rsplit("/", 1)[0], tree) + if node.type != "group": + collector.collect_and_log( + not_visited_key.rsplit("/", 1)[0], + ValidationProblem.AttributeForNonExistingField, + None, + ) + collector.collect_and_log( + not_visited_key, + ValidationProblem.KeyToBeRemoved, + None, + ) + keys_to_remove.append(not_visited_key) + continue + if is_documented(not_visited_key, tree): continue @@ -780,7 +812,6 @@ def startswith_with_variations( not_visited_key, ValidationProblem.MissingDocumentation, None ) - keys_to_remove = check_attributes_of_nonexisting_field(tree) return (not collector.has_validation_problems(), keys_to_remove) diff --git a/tests/dataconverter/test_validation.py b/tests/dataconverter/test_validation.py index 052771503..0e7a6055e 100644 --- a/tests/dataconverter/test_validation.py +++ b/tests/dataconverter/test_validation.py @@ -520,7 +520,7 @@ def listify_template(data_dict: Template): "/ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value", "optional", ), - "There were attributes set for the field /ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value, but the field does not exist.", + "Unit /ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value/@units in dataset without its field /ENTRY[my_entry]/NXODD_name[nxodd_name]/float_value.", id="removed-optional-value-with-attribute-remaining", ), pytest.param( @@ -704,6 +704,70 @@ def listify_template(data_dict: Template): "Field /ENTRY[my_entry]/NXODD_name[nxodd_name]/number_value requires a unit in the unit category NX_ENERGY.", id="missing-unit", ), + pytest.param( + remove_from_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/number_value", + "required", + ), + "Unit /ENTRY[my_entry]/NXODD_name[nxodd_name]/number_value/@units in dataset without its field /ENTRY[my_entry]/NXODD_name[nxodd_name]/number_value.", + id="unit-missing-field", + ), + pytest.param( + remove_from_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name[nxodd_name]/number_value", + "required", + ), + "The attribute /ENTRY[my_entry]/NXODD_name[nxodd_name]/number_value/@units will not be written.", + id="unit-missing-field", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/required_group/illegal_name", + 1, + ), + ( + "Field /ENTRY[my_entry]/required_group/illegal_name written without documentation." + ), + id="add-undocumented-field", + ), + pytest.param( + alter_dict( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/required_group/author", + "author", + ), + "/ENTRY[my_entry]/required_group/author/@illegal", + "illegal_attribute", + ), + ( + "Attribute /ENTRY[my_entry]/required_group/author/@illegal written without documentation." + ), + id="add-undocumented-attribute", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/INSTRUMENT[my_instrument]/BEAM[my_beam]/@default", + "unknown", + ), + "", + id="group-with-only-attributes", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/INSTRUMENT[my_instrument]/BEAM[my_beam]/@illegal", + "unknown", + ), + ( + "Attribute /ENTRY[my_entry]/INSTRUMENT[my_instrument]/BEAM[my_beam]/@illegal written without documentation." + ), + id="group-with-illegal-attributes", + ), pytest.param( alter_dict( TEMPLATE, @@ -726,13 +790,24 @@ def listify_template(data_dict: Template): id="baseclass-missing-unit", ), pytest.param( - remove_from_dict( + alter_dict( TEMPLATE, - "/ENTRY[my_entry]/duration", - "required", + "/ENTRY[my_entry]/collection_time/@illegal", + "s", + ), + ( + "There were attributes set for the field /ENTRY[my_entry]/collection_time, but the field does not exist." + ), + id="baseclass-attribute-missing-field", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/collection_time/@illegal", + "s", ), ( - "There were attributes set for the field /ENTRY[my_entry]/duration, but the field does not exist." + "The attribute /ENTRY[my_entry]/collection_time/@illegal will not be written." ), id="baseclass-attribute-missing-field", ), @@ -761,7 +836,7 @@ def listify_template(data_dict: Template): ( "Field /ENTRY[my_entry]/INSTRUMENT[my_instrument]/SOURCE[my_source]/illegal_name written without documentation." ), - id="add-undocumented-field", + id="baseclass-add-undocumented-field", ), pytest.param( alter_dict( @@ -770,9 +845,32 @@ def listify_template(data_dict: Template): "illegal_attribute", ), ( - "Attribute /ENTRY[my_entry]/INSTRUMENT[my_instrument]/SOURCE[my_source]/type/illegal written without documentation." + "Attribute /ENTRY[my_entry]/INSTRUMENT[my_instrument]/SOURCE[my_source]/type/@illegal written without documentation." ), - id="add-undocumented-attribute", + id="baseclass-add-undocumented-attribute", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/INSTRUMENT[my_instrument]/SOURCE[my_source]/illegal/@units", + "illegal_attribute", + ), + ( + "Unit /ENTRY[my_entry]/INSTRUMENT[my_instrument]/SOURCE[my_source]/illegal/@units " + "in dataset without its field /ENTRY[my_entry]/INSTRUMENT[my_instrument]/SOURCE[my_source]/illegal." + ), + id="baseclass-add-unit-of-missing-undocumented-field", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/INSTRUMENT[my_instrument]/SOURCE[my_source]/illegal/@units", + "illegal_attribute", + ), + ( + "The attribute /ENTRY[my_entry]/INSTRUMENT[my_instrument]/SOURCE[my_source]/illegal/@units will not be written." + ), + id="baseclass-add-unit-of-missing-undocumented-field", ), ], ) @@ -804,6 +902,7 @@ def format_error_message(msg: str) -> str: "array-of-float-instead-of-float", "numpy-chararray", "removed-optional-value", + "group-with-only-attributes", ): with caplog.at_level(logging.WARNING): assert validate_dict_against("NXtest", data_dict)[0] From 3c28c1fbb7871221af951dd1f6caf39314b68955 Mon Sep 17 00:00:00 2001 From: rettigl Date: Wed, 12 Mar 2025 22:38:46 +0100 Subject: [PATCH 36/36] add case and tests for undocumented units --- src/pynxtools/dataconverter/helpers.py | 2 +- src/pynxtools/dataconverter/validation.py | 9 ++++++++ tests/dataconverter/test_validation.py | 26 +++++++++++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/pynxtools/dataconverter/helpers.py b/src/pynxtools/dataconverter/helpers.py index e6851033d..3cc5ce652 100644 --- a/src/pynxtools/dataconverter/helpers.py +++ b/src/pynxtools/dataconverter/helpers.py @@ -81,7 +81,7 @@ def _log(self, path: str, log_type: ValidationProblem, value: Optional[Any], *ar if log_type == ValidationProblem.UnitWithoutDocumentation: logger.warning( - f"The unit, {path} = {value}, is being written but has no documentation" + f"The unit, {path} = {value}, is being written but has no documentation." ) elif log_type == ValidationProblem.InvalidEnum: logger.warning( diff --git a/src/pynxtools/dataconverter/validation.py b/src/pynxtools/dataconverter/validation.py index 543eb3ce6..4c4d35cc8 100644 --- a/src/pynxtools/dataconverter/validation.py +++ b/src/pynxtools/dataconverter/validation.py @@ -781,6 +781,15 @@ def startswith_with_variations( None, ) keys_to_remove.append(not_visited_key) + else: + # check that parent has units + node = add_best_matches_for(not_visited_key.rsplit("/", 1)[0], tree) + if node.unit is None: + collector.collect_and_log( + not_visited_key, + ValidationProblem.UnitWithoutDocumentation, + mapping[not_visited_key], + ) # parent key will be checked on its own if it exists, because it is in the list continue diff --git a/tests/dataconverter/test_validation.py b/tests/dataconverter/test_validation.py index 0e7a6055e..b5a309131 100644 --- a/tests/dataconverter/test_validation.py +++ b/tests/dataconverter/test_validation.py @@ -768,6 +768,17 @@ def listify_template(data_dict: Template): ), id="group-with-illegal-attributes", ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/optional_parent/required_child/@units", + "s", + ), + ( + "The unit, /ENTRY[my_entry]/optional_parent/required_child/@units = s, is being written but has no documentation." + ), + id="field-with-illegal-unit", + ), pytest.param( alter_dict( TEMPLATE, @@ -872,6 +883,21 @@ def listify_template(data_dict: Template): ), id="baseclass-add-unit-of-missing-undocumented-field", ), + pytest.param( + alter_dict( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/required_group/author", + "author", + ), + "/ENTRY[my_entry]/required_group/author/@units", + "s", + ), + ( + "The unit, /ENTRY[my_entry]/required_group/author/@units = s, is being written but has no documentation." + ), + id="baseclass-field-with-illegal-unit", + ), ], ) def test_validate_data_dict(caplog, data_dict, error_message, request):