From 4a292517859557dfa92c57ef005106c657d8be2f Mon Sep 17 00:00:00 2001 From: rettigl Date: Tue, 4 Mar 2025 19:28:13 +0100 Subject: [PATCH 1/2] 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 2/2] 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.