From a231d668ee48f186c1463dcfed8f85263f29289a Mon Sep 17 00:00:00 2001 From: Rubel Date: Sun, 3 Dec 2023 14:12:29 +0100 Subject: [PATCH 1/8] Adding logger in global level. --- pynxtools/dataconverter/convert.py | 53 +++++++++++++++-------------- pynxtools/dataconverter/helpers.py | 27 +++++++++------ pynxtools/dataconverter/logger.py | 25 ++++++++++++++ tests/dataconverter/test_helpers.py | 15 ++++++-- 4 files changed, 82 insertions(+), 38 deletions(-) create mode 100644 pynxtools/dataconverter/logger.py diff --git a/pynxtools/dataconverter/convert.py b/pynxtools/dataconverter/convert.py index 46c9af7eb..1484a6e82 100644 --- a/pynxtools/dataconverter/convert.py +++ b/pynxtools/dataconverter/convert.py @@ -33,6 +33,7 @@ from pynxtools.dataconverter.writer import Writer from pynxtools.dataconverter.template import Template from pynxtools.nexus import nexus +from pynxtools.dataconverter.logger import logger as pynx_logger if sys.version_info >= (3, 10): from importlib.metadata import entry_points @@ -40,12 +41,6 @@ from importlib_metadata import entry_points -logger = logging.getLogger(__name__) # pylint: disable=C0103 -UNDOCUMENTED = 9 -logger.setLevel(logging.INFO) -logger.addHandler(logging.StreamHandler(sys.stdout)) - - def get_reader(reader_name) -> BaseReader: """Helper function to get the reader object from it's given name""" path_prefix = f"{os.path.dirname(__file__)}{os.sep}" if os.path.dirname(__file__) else "" @@ -123,6 +118,7 @@ def get_nxdl_root_and_path(nxdl: str): def transfer_data_into_template(input_file, reader, nxdl_name, nxdl_root: Optional[ET.Element] = None, + logger: logging.Logger = pynx_logger, **kwargs): """Transfer parse and merged data from input experimental file, config file and eln. @@ -139,6 +135,8 @@ def transfer_data_into_template(input_file, Root name of nxdl file, e.g. NXmpes from NXmpes.nxdl.xml nxdl_root : ET.element Root element of nxdl file, otherwise provide nxdl_name + logger: looging.Logger + Logger to get log massages. Returns ------- @@ -156,10 +154,10 @@ def transfer_data_into_template(input_file, input_file = (input_file,) bulletpoint = "\n\u2022 " - logger.info("Using %s reader to convert the given files: %s ", - reader, - bulletpoint.join((" ", *input_file))) - + logger.info("Using %s reader reader to convert the given files: %s ", + reader, bulletpoint.join((' ', *input_file))) + # logger.info(f"Using {reader} reader to convert the given files: " + # f"{bulletpoint.join((' ', *input_file))}.") data_reader = get_reader(reader) if not (nxdl_name in data_reader.supported_nxdls or "*" in data_reader.supported_nxdls): raise NotImplementedError("The chosen NXDL isn't supported by the selected reader.") @@ -169,7 +167,7 @@ def transfer_data_into_template(input_file, file_paths=input_file, **kwargs ) - helpers.validate_data_dict(template, data, nxdl_root) + helpers.validate_data_dict(template, data, nxdl_root, logger=logger) return data @@ -181,6 +179,7 @@ def convert(input_file: Tuple[str, ...], generate_template: bool = False, fair: bool = False, undocumented: bool = False, + logger: logging.Logger = pynx_logger, **kwargs): """The conversion routine that takes the input parameters and calls the necessary functions. @@ -201,6 +200,8 @@ def convert(input_file: Tuple[str, ...], in the template. undocumented : bool, default False If True, an undocumented warning is given. + logger: looging.Logger + Logger to get log massages. Returns ------- @@ -217,25 +218,27 @@ def convert(input_file: Tuple[str, ...], data = transfer_data_into_template(input_file=input_file, reader=reader, nxdl_name=nxdl, nxdl_root=nxdl_root, - **kwargs) - if undocumented: - logger.setLevel(UNDOCUMENTED) + logger=logger, **kwargs) + if fair and data.undocumented.keys(): logger.warning("There are undocumented paths in the template. This is not acceptable!") return - - for path in data.undocumented.keys(): - if "/@default" in path: - continue - logger.log( - UNDOCUMENTED, - "The path, %s, is being written but has no documentation.", - path - ) - helpers.add_default_root_attributes(data=data, filename=os.path.basename(output)) + if undocumented: + for path in data.undocumented.keys(): + if "/@default" in path: + continue + logger.info( + "NO DOCUMENTATION: The path, %s, is being written but has no documentation.", + path) + # logger.info( + # f"NO DOCUMENTATION: The path, {path}, is being written but has no documentation.") + + helpers.add_default_root_attributes(data=data, filename=os.path.basename(output), + logger=logger) Writer(data=data, nxdl_f_path=nxdl_f_path, output_path=output).write() - logger.info("The output file generated: %s", output) + logger.info("The output file generated: %s ", output) + # logger.info(f"The output file generated: {output}") def parse_params_file(params_file): diff --git a/pynxtools/dataconverter/helpers.py b/pynxtools/dataconverter/helpers.py index 57d526f4b..70a469251 100644 --- a/pynxtools/dataconverter/helpers.py +++ b/pynxtools/dataconverter/helpers.py @@ -32,8 +32,7 @@ from pynxtools import get_nexus_version, get_nexus_version_hash from pynxtools.nexus import nexus from pynxtools.nexus.nexus import NxdlAttributeError - -logger = logging.getLogger(__name__) +from pynxtools.dataconverter.logger import logger as pynx_logger def is_a_lone_group(xml_element) -> bool: @@ -447,7 +446,9 @@ def does_group_exist(path_to_group, data): return False -def ensure_all_required_fields_exist(template, data, nxdl_root): +# pylint: disable=W1203 +def ensure_all_required_fields_exist(template, data, + nxdl_root, logger=pynx_logger): """Checks whether all the required fields are in the returned data object.""" for path in template["required"]: entry_name = get_name_from_data_dict_entry(path[path.rindex('/') + 1:]) @@ -461,15 +462,20 @@ def ensure_all_required_fields_exist(template, data, nxdl_root): opt_parent = check_for_optional_parent(path, nxdl_root) if opt_parent != "<>": if does_group_exist(opt_parent, data) and not does_group_exist(renamed_path, data): - raise ValueError(f"The required group, {path}, hasn't been supplied" - f" while its optional parent, {path}, is supplied.") + logger.warning("The required group, %s, hasn't been supplied" + " while its optional parent, %s, is supplied.", path, + opt_parent) + # logger.warning("The required group, {path}, hasn't been supplied" + # f" while its optional parent, {path}, is supplied.") continue if not does_group_exist(renamed_path, data): raise ValueError(f"The required group, {path}, hasn't been supplied.") continue if not is_path_in_data_dict or data[renamed_path] is None: - raise ValueError(f"The data entry corresponding to {path} is required " - f"and hasn't been supplied by the reader.") + logger.warning("The data entry corresponding to %s is required " + "and hasn't been supplied by the reader.", path) + # logger.warning(f"The data entry corresponding to {path} is required " + # f"and hasn't been supplied by the reader.") def try_undocumented(data, nxdl_root: ET.Element): @@ -498,7 +504,8 @@ def try_undocumented(data, nxdl_root: ET.Element): pass -def validate_data_dict(template, data, nxdl_root: ET.Element): +def validate_data_dict(template, data, + nxdl_root: ET.Element, logger=pynx_logger): """Checks whether all the required paths from the template are returned in data dict.""" assert nxdl_root is not None, "The NXDL file hasn't been loaded." @@ -507,7 +514,7 @@ def validate_data_dict(template, data, nxdl_root: ET.Element): nxdl_path_to_elm: dict = {} # Make sure all required fields exist. - ensure_all_required_fields_exist(template, data, nxdl_root) + ensure_all_required_fields_exist(template, data, nxdl_root, logger) try_undocumented(data, nxdl_root) for path in data.get_documented().keys(): @@ -590,7 +597,7 @@ def convert_to_hill(atoms_typ): return atom_list + list(atoms_typ) -def add_default_root_attributes(data, filename): +def add_default_root_attributes(data, filename, logger=pynx_logger): """ Takes a dict/Template and adds NXroot fields/attributes that are inherently available """ diff --git a/pynxtools/dataconverter/logger.py b/pynxtools/dataconverter/logger.py new file mode 100644 index 000000000..13a4cf4ea --- /dev/null +++ b/pynxtools/dataconverter/logger.py @@ -0,0 +1,25 @@ +"""Logger for pynxtools""" +# +# Copyright The NOMAD Authors. +# +# This file is part of NOMAD. See https://nomad-lab.eu for further info. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +import logging + +logger = logging.getLogger("pynxtools") + +# Lowest level log allows to other levels erros, crittical, info and debug +logger.setLevel(logging.DEBUG) diff --git a/tests/dataconverter/test_helpers.py b/tests/dataconverter/test_helpers.py index 421f8ce9b..4e91c78bf 100644 --- a/tests/dataconverter/test_helpers.py +++ b/tests/dataconverter/test_helpers.py @@ -25,6 +25,7 @@ import numpy as np from pynxtools.dataconverter import helpers +from pynxtools.dataconverter.logger import logger as pynx_logger from pynxtools.dataconverter.template import Template @@ -189,6 +190,7 @@ def fixture_filled_test_data(template, tmp_path): TEMPLATE["optional"]["/@default"] = "Some NXroot attribute" +# pylint: disable=too-many-arguments @pytest.mark.parametrize("data_dict,error_message", [ pytest.param( alter_dict(TEMPLATE, "/ENTRY[my_entry]/NXODD_name/int_value", "not_a_num"), @@ -310,7 +312,7 @@ def fixture_filled_test_data(template, tmp_path): id="opt-group-completely-removed" ), ]) -def test_validate_data_dict(data_dict, error_message, template, nxdl_root, request): +def test_validate_data_dict(caplog, data_dict, error_message, template, nxdl_root, request): """Unit test for the data validation routine""" if request.node.callspec.id in ("valid-data-dict", "lists", @@ -322,10 +324,17 @@ def test_validate_data_dict(data_dict, error_message, template, nxdl_root, reque "link-dict-instead-of-bool", "allow-required-and-empty-group", "opt-group-completely-removed"): - helpers.validate_data_dict(template, data_dict, nxdl_root) + helpers.validate_data_dict(template, data_dict, nxdl_root, logger=pynx_logger) + # Missing required fields + elif request.node.callspec.id in ("empty-required-field", + "req-group-in-opt-parent-removed" + ): + captured_logs = caplog.records + helpers.validate_data_dict(template, data_dict, nxdl_root, pynx_logger) + assert any(error_message in rec.message for rec in captured_logs) else: with pytest.raises(Exception) as execinfo: - helpers.validate_data_dict(template, data_dict, nxdl_root) + helpers.validate_data_dict(template, data_dict, nxdl_root, pynx_logger) assert (error_message) == str(execinfo.value) From f558f7d97ad4078b08362e338f515626c8b621ab Mon Sep 17 00:00:00 2001 From: Rubel Date: Sun, 3 Dec 2023 15:08:10 +0100 Subject: [PATCH 2/8] CI --- pynxtools/dataconverter/helpers.py | 1 - tests/dataconverter/test_helpers.py | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/pynxtools/dataconverter/helpers.py b/pynxtools/dataconverter/helpers.py index 70a469251..dbe59b4a1 100644 --- a/pynxtools/dataconverter/helpers.py +++ b/pynxtools/dataconverter/helpers.py @@ -22,7 +22,6 @@ import re import xml.etree.ElementTree as ET from datetime import datetime, timezone -import logging import json import numpy as np diff --git a/tests/dataconverter/test_helpers.py b/tests/dataconverter/test_helpers.py index 4e91c78bf..de19329e9 100644 --- a/tests/dataconverter/test_helpers.py +++ b/tests/dataconverter/test_helpers.py @@ -302,8 +302,7 @@ def fixture_filled_test_data(template, tmp_path): "required" ), ("The required group, /ENTRY[entry]/optional_parent/req_group_in_opt_group, hasn't been " - "supplied while its optional parent, /ENTRY[entry]/optional_parent/" - "req_group_in_opt_group, is supplied."), + "supplied while its optional parent, /ENTRY[entry]/optional_parent, is supplied."), id="req-group-in-opt-parent-removed" ), pytest.param( From b78da32c04603ffbca9482559a58e4973e0543a4 Mon Sep 17 00:00:00 2001 From: Rubel Date: Sun, 3 Dec 2023 15:46:33 +0100 Subject: [PATCH 3/8] removing unnecessary codes. --- pynxtools/dataconverter/convert.py | 7 +------ pynxtools/dataconverter/helpers.py | 5 ----- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/pynxtools/dataconverter/convert.py b/pynxtools/dataconverter/convert.py index 1484a6e82..ae1febf7f 100644 --- a/pynxtools/dataconverter/convert.py +++ b/pynxtools/dataconverter/convert.py @@ -156,8 +156,7 @@ def transfer_data_into_template(input_file, bulletpoint = "\n\u2022 " logger.info("Using %s reader reader to convert the given files: %s ", reader, bulletpoint.join((' ', *input_file))) - # logger.info(f"Using {reader} reader to convert the given files: " - # f"{bulletpoint.join((' ', *input_file))}.") + data_reader = get_reader(reader) if not (nxdl_name in data_reader.supported_nxdls or "*" in data_reader.supported_nxdls): raise NotImplementedError("The chosen NXDL isn't supported by the selected reader.") @@ -209,7 +208,6 @@ def convert(input_file: Tuple[str, ...], """ nxdl_root, nxdl_f_path = get_nxdl_root_and_path(nxdl) - if generate_template: template = Template() helpers.generate_template_from_nxdl(nxdl_root, template) @@ -230,15 +228,12 @@ def convert(input_file: Tuple[str, ...], logger.info( "NO DOCUMENTATION: The path, %s, is being written but has no documentation.", path) - # logger.info( - # f"NO DOCUMENTATION: The path, {path}, is being written but has no documentation.") helpers.add_default_root_attributes(data=data, filename=os.path.basename(output), logger=logger) Writer(data=data, nxdl_f_path=nxdl_f_path, output_path=output).write() logger.info("The output file generated: %s ", output) - # logger.info(f"The output file generated: {output}") def parse_params_file(params_file): diff --git a/pynxtools/dataconverter/helpers.py b/pynxtools/dataconverter/helpers.py index dbe59b4a1..b317ac468 100644 --- a/pynxtools/dataconverter/helpers.py +++ b/pynxtools/dataconverter/helpers.py @@ -464,8 +464,6 @@ def ensure_all_required_fields_exist(template, data, logger.warning("The required group, %s, hasn't been supplied" " while its optional parent, %s, is supplied.", path, opt_parent) - # logger.warning("The required group, {path}, hasn't been supplied" - # f" while its optional parent, {path}, is supplied.") continue if not does_group_exist(renamed_path, data): raise ValueError(f"The required group, {path}, hasn't been supplied.") @@ -473,9 +471,6 @@ def ensure_all_required_fields_exist(template, data, if not is_path_in_data_dict or data[renamed_path] is None: logger.warning("The data entry corresponding to %s is required " "and hasn't been supplied by the reader.", path) - # logger.warning(f"The data entry corresponding to {path} is required " - # f"and hasn't been supplied by the reader.") - def try_undocumented(data, nxdl_root: ET.Element): """Tries to move entries used that are from base classes but not in AppDef""" From cdf74bf3c1c46feda3fc114f115cd61de84cd751 Mon Sep 17 00:00:00 2001 From: Rubel Date: Sun, 3 Dec 2023 15:54:48 +0100 Subject: [PATCH 4/8] CI --- pynxtools/dataconverter/helpers.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pynxtools/dataconverter/helpers.py b/pynxtools/dataconverter/helpers.py index b317ac468..4317361b7 100644 --- a/pynxtools/dataconverter/helpers.py +++ b/pynxtools/dataconverter/helpers.py @@ -472,6 +472,7 @@ def ensure_all_required_fields_exist(template, data, logger.warning("The data entry corresponding to %s is required " "and hasn't been supplied by the reader.", path) + def try_undocumented(data, nxdl_root: ET.Element): """Tries to move entries used that are from base classes but not in AppDef""" for path in list(data.undocumented): From 4c5ae50330b8e18300c7cee37db4cda057cfa6d0 Mon Sep 17 00:00:00 2001 From: Rubel Date: Tue, 5 Dec 2023 09:26:09 +0100 Subject: [PATCH 5/8] updating f-string. --- pynxtools/dataconverter/convert.py | 11 +++++------ pynxtools/dataconverter/helpers.py | 14 ++++++-------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/pynxtools/dataconverter/convert.py b/pynxtools/dataconverter/convert.py index ae1febf7f..75dd040ac 100644 --- a/pynxtools/dataconverter/convert.py +++ b/pynxtools/dataconverter/convert.py @@ -154,8 +154,8 @@ def transfer_data_into_template(input_file, input_file = (input_file,) bulletpoint = "\n\u2022 " - logger.info("Using %s reader reader to convert the given files: %s ", - reader, bulletpoint.join((' ', *input_file))) + logger.info(f"Using {reader} reader reader to convert the given files:" + f" {bulletpoint.join((' ', *input_file))}") data_reader = get_reader(reader) if not (nxdl_name in data_reader.supported_nxdls or "*" in data_reader.supported_nxdls): @@ -170,7 +170,7 @@ def transfer_data_into_template(input_file, return data -# pylint: disable=too-many-arguments,too-many-locals +# pylint: disable=too-many-arguments,too-many-locals,W1203 def convert(input_file: Tuple[str, ...], reader: str, nxdl: str, @@ -226,14 +226,13 @@ def convert(input_file: Tuple[str, ...], if "/@default" in path: continue logger.info( - "NO DOCUMENTATION: The path, %s, is being written but has no documentation.", - path) + f"NO DOCUMENTATION: The path, {path}, is being written but has no documentation.") helpers.add_default_root_attributes(data=data, filename=os.path.basename(output), logger=logger) Writer(data=data, nxdl_f_path=nxdl_f_path, output_path=output).write() - logger.info("The output file generated: %s ", output) + logger.info(f"The output file generated: {output}.") def parse_params_file(params_file): diff --git a/pynxtools/dataconverter/helpers.py b/pynxtools/dataconverter/helpers.py index 4317361b7..03e85dc5e 100644 --- a/pynxtools/dataconverter/helpers.py +++ b/pynxtools/dataconverter/helpers.py @@ -461,16 +461,15 @@ def ensure_all_required_fields_exist(template, data, opt_parent = check_for_optional_parent(path, nxdl_root) if opt_parent != "<>": if does_group_exist(opt_parent, data) and not does_group_exist(renamed_path, data): - logger.warning("The required group, %s, hasn't been supplied" - " while its optional parent, %s, is supplied.", path, - opt_parent) + logger.warning(f"The required group, {path}, hasn't been supplied" + f" while its optional parent, {opt_parent}, is supplied.") continue if not does_group_exist(renamed_path, data): raise ValueError(f"The required group, {path}, hasn't been supplied.") continue if not is_path_in_data_dict or data[renamed_path] is None: - logger.warning("The data entry corresponding to %s is required " - "and hasn't been supplied by the reader.", path) + logger.warning(f"The data entry corresponding to {path} is required " + f"and hasn't been supplied by the reader.",) def try_undocumented(data, nxdl_root: ET.Element): @@ -599,9 +598,8 @@ def add_default_root_attributes(data, filename, logger=pynx_logger): def update_and_warn(key: str, value: str): if key in data and data[key] != value: logger.warning( - "The NXroot entry '%s' (value: %s) should not be populated by the reader. " - "This is overwritten by the actually used value '%s'", - key, data[key], value + f"The NXroot entry {key} (value: {data[key]}) should not be populated by " + f"the reader. This is overwritten by the actually used value {value}" ) data[key] = value From 0bdb8feb02374a0b6c7a608dd7a92bb70d4f9161 Mon Sep 17 00:00:00 2001 From: Rubel Date: Tue, 5 Dec 2023 09:44:39 +0100 Subject: [PATCH 6/8] test. --- pynxtools/dataconverter/helpers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pynxtools/dataconverter/helpers.py b/pynxtools/dataconverter/helpers.py index 03e85dc5e..3d838cb28 100644 --- a/pynxtools/dataconverter/helpers.py +++ b/pynxtools/dataconverter/helpers.py @@ -598,8 +598,8 @@ def add_default_root_attributes(data, filename, logger=pynx_logger): def update_and_warn(key: str, value: str): if key in data and data[key] != value: logger.warning( - f"The NXroot entry {key} (value: {data[key]}) should not be populated by " - f"the reader. This is overwritten by the actually used value {value}" + f"The NXroot entry '{key}' (value: {data[key]}) should not be populated by " + f"the reader. This is overwritten by the actually used value '{value}'" ) data[key] = value From 24f4934ed7ba623bb6feea0ab9ad9433d0f86a2d Mon Sep 17 00:00:00 2001 From: domna Date: Tue, 19 Dec 2023 20:22:22 +0100 Subject: [PATCH 7/8] Ruff formatting fixing test. --- pynxtools/dataconverter/convert.py | 58 +++-- pynxtools/dataconverter/helpers.py | 35 ++-- tests/dataconverter/test_helpers.py | 314 ++++++++++++++++------------ 3 files changed, 233 insertions(+), 174 deletions(-) diff --git a/pynxtools/dataconverter/convert.py b/pynxtools/dataconverter/convert.py index 2f8637713..767b58769 100644 --- a/pynxtools/dataconverter/convert.py +++ b/pynxtools/dataconverter/convert.py @@ -140,11 +140,14 @@ def get_nxdl_root_and_path(nxdl: str): return ET.parse(nxdl_f_path).getroot(), nxdl_f_path -def transfer_data_into_template(input_file, - reader, nxdl_name, - nxdl_root: Optional[ET.Element] = None, - logger: logging.Logger = pynx_logger, - **kwargs): +def transfer_data_into_template( + input_file, + reader, + nxdl_name, + nxdl_root: Optional[ET.Element] = None, + logger: logging.Logger = pynx_logger, + **kwargs, +): """Transfer parse and merged data from input experimental file, config file and eln. Experimental and eln files will be parsed and finally will be merged into template. @@ -179,8 +182,10 @@ def transfer_data_into_template(input_file, input_file = (input_file,) bulletpoint = "\n\u2022 " - logger.info(f"Using {reader} reader reader to convert the given files:" - f" {bulletpoint.join((' ', *input_file))}") + logger.info( + f"Using {reader} reader reader to convert the given files:" + f" {bulletpoint.join((' ', *input_file))}" + ) data_reader = get_reader(reader) if not ( @@ -198,15 +203,17 @@ def transfer_data_into_template(input_file, # pylint: disable=too-many-arguments,too-many-locals,W1203 -def convert(input_file: Tuple[str, ...], - reader: str, - nxdl: str, - output: str, - generate_template: bool = False, - fair: bool = False, - undocumented: bool = False, - logger: logging.Logger = pynx_logger, - **kwargs): +def convert( + input_file: Tuple[str, ...], + reader: str, + nxdl: str, + output: str, + generate_template: bool = False, + fair: bool = False, + undocumented: bool = False, + logger: logging.Logger = pynx_logger, + **kwargs, +): """The conversion routine that takes the input parameters and calls the necessary functions. Parameters @@ -241,9 +248,14 @@ def convert(input_file: Tuple[str, ...], logger.info(template) return - data = transfer_data_into_template(input_file=input_file, reader=reader, - nxdl_name=nxdl, nxdl_root=nxdl_root, - logger=logger, **kwargs) + data = transfer_data_into_template( + input_file=input_file, + reader=reader, + nxdl_name=nxdl, + nxdl_root=nxdl_root, + logger=logger, + **kwargs, + ) if fair and data.undocumented.keys(): logger.warning( @@ -255,10 +267,12 @@ def convert(input_file: Tuple[str, ...], if "/@default" in path: continue logger.info( - f"NO DOCUMENTATION: The path, {path}, is being written but has no documentation.") + f"NO DOCUMENTATION: The path, {path}, is being written but has no documentation." + ) - helpers.add_default_root_attributes(data=data, filename=os.path.basename(output), - logger=logger) + helpers.add_default_root_attributes( + data=data, filename=os.path.basename(output), logger=logger + ) Writer(data=data, nxdl_f_path=nxdl_f_path, output_path=output).write() logger.info(f"The output file generated: {output}.") diff --git a/pynxtools/dataconverter/helpers.py b/pynxtools/dataconverter/helpers.py index 8ae69b87f..002c33a61 100644 --- a/pynxtools/dataconverter/helpers.py +++ b/pynxtools/dataconverter/helpers.py @@ -17,21 +17,20 @@ # """Helper functions commonly used by the convert routine.""" -from typing import List, Optional, Any -from typing import Tuple, Callable, Union +import json import re import xml.etree.ElementTree as ET from datetime import datetime, timezone -import json +from typing import Any, Callable, List, Optional, Tuple, Union +import h5py import numpy as np from ase.data import chemical_symbols -import h5py from pynxtools import get_nexus_version, get_nexus_version_hash +from pynxtools.dataconverter.logger import logger as pynx_logger from pynxtools.nexus import nexus from pynxtools.nexus.nexus import NxdlAttributeError -from pynxtools.dataconverter.logger import logger as pynx_logger def is_a_lone_group(xml_element) -> bool: @@ -488,8 +487,7 @@ def does_group_exist(path_to_group, data): # pylint: disable=W1203 -def ensure_all_required_fields_exist(template, data, - nxdl_root, logger=pynx_logger): +def ensure_all_required_fields_exist(template, data, nxdl_root, logger=pynx_logger): """Checks whether all the required fields are in the returned data object.""" for path in template["required"]: entry_name = get_name_from_data_dict_entry(path[path.rindex("/") + 1 :]) @@ -502,16 +500,22 @@ def ensure_all_required_fields_exist(template, data, if path in template["lone_groups"]: opt_parent = check_for_optional_parent(path, nxdl_root) if opt_parent != "<>": - if does_group_exist(opt_parent, data) and not does_group_exist(renamed_path, data): - logger.warning(f"The required group, {path}, hasn't been supplied" - f" while its optional parent, {opt_parent}, is supplied.") + if does_group_exist(opt_parent, data) and not does_group_exist( + renamed_path, data + ): + logger.warning( + f"The required group, {path}, hasn't been supplied" + f" while its optional parent, {opt_parent}, is supplied." + ) continue if not does_group_exist(renamed_path, data): - raise ValueError(f"The required group, {path}, hasn't been supplied.") - continue + logger.warning(f"The required group, {path}, hasn't been supplied.") + continue if not is_path_in_data_dict or data[renamed_path] is None: - logger.warning(f"The data entry corresponding to {path} is required " - f"and hasn't been supplied by the reader.",) + logger.warning( + f"The data entry corresponding to {path} is required " + f"and hasn't been supplied by the reader.", + ) def try_undocumented(data, nxdl_root: ET.Element): @@ -540,8 +544,7 @@ def try_undocumented(data, nxdl_root: ET.Element): pass -def validate_data_dict(template, data, - nxdl_root: ET.Element, logger=pynx_logger): +def validate_data_dict(template, data, nxdl_root: ET.Element, logger=pynx_logger): """Checks whether all the required paths from the template are returned in data dict.""" assert nxdl_root is not None, "The NXDL file hasn't been loaded." diff --git a/tests/dataconverter/test_helpers.py b/tests/dataconverter/test_helpers.py index efa77b4a2..cc528f8b7 100644 --- a/tests/dataconverter/test_helpers.py +++ b/tests/dataconverter/test_helpers.py @@ -17,12 +17,13 @@ # """Test cases for the helper functions used by the DataConverter.""" -import xml.etree.ElementTree as ET -import os import logging -from setuptools import distutils -import pytest +import os +import xml.etree.ElementTree as ET + import numpy as np +import pytest +from setuptools import distutils from pynxtools.dataconverter import helpers from pynxtools.dataconverter.logger import logger as pynx_logger @@ -215,104 +216,132 @@ def fixture_filled_test_data(template, tmp_path): # pylint: disable=too-many-arguments -@pytest.mark.parametrize("data_dict,error_message", [ - pytest.param( - alter_dict(TEMPLATE, "/ENTRY[my_entry]/NXODD_name/int_value", "not_a_num"), - ("The value at /ENTRY[my_entry]/NXODD_name/in" - "t_value should be of Python type: (, , )," - " as defined in the NXDL as NX_INT."), - id="string-instead-of-int"), - pytest.param( - alter_dict(TEMPLATE, "/ENTRY[my_entry]/NXODD_name/bool_value", "NOT_TRUE_OR_FALSE"), - ("The value at /ENTRY[my_entry]/NXODD_name/bool_value sh" - "ould be of Python type: (, , ), as defined in the NXDL as NX_BOOLEAN."), - id="string-instead-of-int"), - pytest.param( - alter_dict(TEMPLATE, "/ENTRY[my_entry]/NXODD_name/int_value", {"link": "/a-link"}), - (""), - id="link-dict-instead-of-bool"), - pytest.param( - alter_dict(TEMPLATE, "/ENTRY[my_entry]/NXODD_name/posint_value", -1), - ("The value at /ENTRY[my_entry]/NXODD_name/posint_value " - "should be a positive int."), - id="negative-posint"), - pytest.param( - alter_dict(TEMPLATE, "/ENTRY[my_entry]/NXODD_name/char_value", 3), - ("The value at /ENTRY[my_entry]/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/float_value", None), - "", - id="empty-optional-field"), - pytest.param( - set_to_none_in_dict(TEMPLATE, "/ENTRY[my_entry]/NXODD_name/bool_value", "required"), - ("The data entry corresponding to /ENTRY[entry]/NXODD_name/bool_value is" - " required and hasn't been supplied by the reader."), - id="empty-required-field"), - pytest.param( - alter_dict(TEMPLATE, - "/ENTRY[my_entry]/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/date_value", - "2022-01-22T12:14:12.05018Z"), - "", - id="UTC-with-Z"), - pytest.param( - alter_dict(TEMPLATE, - "/ENTRY[my_entry]/NXODD_name/date_value", - "2022-01-22T12:14:12.05018-00:00"), - "The date at /ENTRY[my_entry]/NXODD_name/date_value 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/type", "Wrong option"), - ("The value at /ENTRY[my_entry]/NXODD_name/type should be on of the following" - " strings: [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, /ENTRY[my_entry]/optional_parent/optional_child, has an " - "optional parent, /ENTRY[entry]/optional_parent, with required children set" - ". Either provide no children for /ENTRY[entry]/optional_parent or provide " - "all required ones."), - id="atleast-one-required-child-not-provided-optional-parent"), - 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[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[entry]/required_group2, hasn't been supplied.", - id="missing-empty-yet-required-group2"), - pytest.param( - alter_dict( +@pytest.mark.parametrize( + "data_dict,error_message", + [ + pytest.param( + alter_dict(TEMPLATE, "/ENTRY[my_entry]/NXODD_name/int_value", "not_a_num"), + ( + "The value at /ENTRY[my_entry]/NXODD_name/in" + "t_value should be of Python type: (, , )," + " as defined in the NXDL as NX_INT." + ), + id="string-instead-of-int", + ), + pytest.param( + alter_dict( + TEMPLATE, "/ENTRY[my_entry]/NXODD_name/bool_value", "NOT_TRUE_OR_FALSE" + ), + ( + "The value at /ENTRY[my_entry]/NXODD_name/bool_value sh" + "ould be of Python type: (, , ), as defined in the NXDL as NX_BOOLEAN." + ), + id="string-instead-of-int", + ), + pytest.param( + alter_dict( + TEMPLATE, "/ENTRY[my_entry]/NXODD_name/int_value", {"link": "/a-link"} + ), + (""), + id="link-dict-instead-of-bool", + ), + pytest.param( + alter_dict(TEMPLATE, "/ENTRY[my_entry]/NXODD_name/posint_value", -1), + ( + "The value at /ENTRY[my_entry]/NXODD_name/posint_value " + "should be a positive int." + ), + id="negative-posint", + ), + pytest.param( + alter_dict(TEMPLATE, "/ENTRY[my_entry]/NXODD_name/char_value", 3), + ( + "The value at /ENTRY[my_entry]/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/float_value", None), + "", + id="empty-optional-field", + ), + pytest.param( + set_to_none_in_dict( + TEMPLATE, "/ENTRY[my_entry]/NXODD_name/bool_value", "required" + ), + ( + "The data entry corresponding to /ENTRY[entry]/NXODD_name/bool_value is" + " required and hasn't been supplied by the reader." + ), + id="empty-required-field", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/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/date_value", + "2022-01-22T12:14:12.05018Z", + ), + "", + id="UTC-with-Z", + ), + pytest.param( + alter_dict( + TEMPLATE, + "/ENTRY[my_entry]/NXODD_name/date_value", + "2022-01-22T12:14:12.05018-00:00", + ), + "The date at /ENTRY[my_entry]/NXODD_name/date_value 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/type", "Wrong option"), + ( + "The value at /ENTRY[my_entry]/NXODD_name/type should be on of the following" + " strings: [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, /ENTRY[my_entry]/optional_parent/optional_child, has an " + "optional parent, /ENTRY[entry]/optional_parent, with required children set" + ". Either provide no children for /ENTRY[entry]/optional_parent or provide " + "all required ones." + ), + id="atleast-one-required-child-not-provided-optional-parent", + ), + 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[entry]/required_group, hasn't been supplied.", id="missing-empty-yet-required-group", @@ -322,41 +351,54 @@ def fixture_filled_test_data(template, tmp_path): "The required group, /ENTRY[entry]/required_group2, hasn't been supplied.", id="missing-empty-yet-required-group2", ), - (""), - 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[entry]/optional_parent/req_group_in_opt_group, hasn't been " - "supplied while its optional parent, /ENTRY[entry]/optional_parent, is supplied."), - id="req-group-in-opt-parent-removed" - ), - pytest.param( - remove_optional_parent(TEMPLATE), - (""), - id="opt-group-completely-removed" - ), -]) -def test_validate_data_dict(caplog, data_dict, error_message, template, nxdl_root, request): - """Unit test for the data validation routine""" - 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", - "int-instead-of-chars", - "link-dict-instead-of-bool", - "allow-required-and-empty-group", - "opt-group-completely-removed"): + pytest.param( + remove_from_dict(TEMPLATE, "/ENTRY[my_entry]/required_group/description"), + "The required group, /ENTRY[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[entry]/optional_parent/req_group_in_opt_group, hasn't been " + "supplied while its optional parent, /ENTRY[entry]/optional_parent, is supplied." + ), + id="req-group-in-opt-parent-removed", + ), + pytest.param( + remove_optional_parent(TEMPLATE), (""), id="opt-group-completely-removed" + ), + ], +) +def test_validate_data_dict( + caplog, data_dict, error_message, template, nxdl_root, request +): + """Unit test for the data validation routine.""" + 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", + "int-instead-of-chars", + "link-dict-instead-of-bool", + "opt-group-completely-removed", + ): helpers.validate_data_dict(template, data_dict, nxdl_root, logger=pynx_logger) - # Missing required fields - elif request.node.callspec.id in ("empty-required-field", - "req-group-in-opt-parent-removed" - ): + # 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 + # logger records captured_logs = caplog.records helpers.validate_data_dict(template, data_dict, nxdl_root, pynx_logger) assert any(error_message in rec.message for rec in captured_logs) From 04dae62164513f7d14b9c5a7845d105dccc8e2da Mon Sep 17 00:00:00 2001 From: Rubel Date: Wed, 20 Dec 2023 16:34:48 +0100 Subject: [PATCH 8/8] CI and test. --- tests/dataconverter/test_helpers.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/dataconverter/test_helpers.py b/tests/dataconverter/test_helpers.py index cc528f8b7..d368fe955 100644 --- a/tests/dataconverter/test_helpers.py +++ b/tests/dataconverter/test_helpers.py @@ -352,7 +352,13 @@ def fixture_filled_test_data(template, tmp_path): id="missing-empty-yet-required-group2", ), pytest.param( - remove_from_dict(TEMPLATE, "/ENTRY[my_entry]/required_group/description"), + alter_dict( + remove_from_dict( + TEMPLATE, "/ENTRY[my_entry]/required_group/description" + ), + "/ENTRY[my_entry]/required_group", + None, + ), "The required group, /ENTRY[entry]/required_group, hasn't been supplied.", id="allow-required-and-empty-group", ),