From c4d1b9cf8fdabe1a9d49ec54b09a78e5b773e28b Mon Sep 17 00:00:00 2001 From: RubelMozumder <32923026+RubelMozumder@users.noreply.github.com> Date: Wed, 20 Dec 2023 16:47:56 +0100 Subject: [PATCH] Adding logger in global level. (#191) * Adding logger in global level. * CI * removing unnecessary codes. * updating f-string. * test. * Ruff formatting --------- Co-authored-by: Florian Dobener --- pynxtools/dataconverter/convert.py | 54 ++++++++++++++++------------- pynxtools/dataconverter/helpers.py | 39 +++++++++------------ pynxtools/dataconverter/logger.py | 25 +++++++++++++ tests/dataconverter/test_helpers.py | 42 +++++++++++++++------- 4 files changed, 100 insertions(+), 60 deletions(-) create mode 100644 pynxtools/dataconverter/logger.py diff --git a/pynxtools/dataconverter/convert.py b/pynxtools/dataconverter/convert.py index a03a9e909..767b58769 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 @@ -48,12 +49,6 @@ def entry_points(group): return [] -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 = ( @@ -146,7 +141,12 @@ def get_nxdl_root_and_path(nxdl: str): def transfer_data_into_template( - input_file, reader, nxdl_name, nxdl_root: Optional[ET.Element] = None, **kwargs + 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. @@ -163,6 +163,8 @@ def transfer_data_into_template( 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 ------- @@ -181,9 +183,8 @@ def transfer_data_into_template( bulletpoint = "\n\u2022 " logger.info( - "Using %s reader to convert the given files: %s ", - reader, - bulletpoint.join((" ", *input_file)), + f"Using {reader} reader reader to convert the given files:" + f" {bulletpoint.join((' ', *input_file))}" ) data_reader = get_reader(reader) @@ -197,11 +198,11 @@ def transfer_data_into_template( data = data_reader().read( # type: ignore[operator] template=Template(template), 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 -# 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, @@ -210,6 +211,7 @@ def convert( 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. @@ -231,6 +233,8 @@ def convert( in the template. undocumented : bool, default False If True, an undocumented warning is given. + logger: looging.Logger + Logger to get log massages. Returns ------- @@ -238,7 +242,6 @@ def convert( """ nxdl_root, nxdl_f_path = get_nxdl_root_and_path(nxdl) - if generate_template: template = Template() helpers.generate_template_from_nxdl(nxdl_root, template) @@ -250,28 +253,29 @@ def convert( reader=reader, nxdl_name=nxdl, nxdl_root=nxdl_root, + logger=logger, **kwargs, ) - if undocumented: - logger.setLevel(UNDOCUMENTED) + if fair and data.undocumented.keys(): logger.warning( "There are undocumented paths in the template. This is not acceptable!" ) return + if undocumented: + for path in data.undocumented.keys(): + if "/@default" in path: + continue + logger.info( + f"NO DOCUMENTATION: The path, {path}, is being written but has no documentation." + ) - 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)) + 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 9805b88eb..002c33a61 100644 --- a/pynxtools/dataconverter/helpers.py +++ b/pynxtools/dataconverter/helpers.py @@ -17,24 +17,21 @@ # """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 logging -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 -logger = logging.getLogger(__name__) - def is_a_lone_group(xml_element) -> bool: """Checks whether a given group XML element has no field or attributes mentioned""" @@ -489,7 +486,8 @@ 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 :]) @@ -505,18 +503,18 @@ def ensure_all_required_fields_exist(template, data, nxdl_root): if does_group_exist(opt_parent, data) and not does_group_exist( renamed_path, data ): - raise ValueError( + logger.warning( f"The required group, {path}, hasn't been supplied" - f" while its optional parent, {path}, is 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: - raise ValueError( + logger.warning( f"The data entry corresponding to {path} is required " - f"and hasn't been supplied by the reader." + f"and hasn't been supplied by the reader.", ) @@ -546,7 +544,7 @@ 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." @@ -555,7 +553,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(): @@ -652,7 +650,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 """ @@ -660,11 +658,8 @@ def add_default_root_attributes(data, filename): 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 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 21f41586d..d368fe955 100644 --- a/tests/dataconverter/test_helpers.py +++ b/tests/dataconverter/test_helpers.py @@ -17,14 +17,16 @@ # """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 from pynxtools.dataconverter.template import Template @@ -213,6 +215,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", [ @@ -354,9 +357,9 @@ def fixture_filled_test_data(template, tmp_path): 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", ), pytest.param( @@ -367,8 +370,7 @@ def fixture_filled_test_data(template, tmp_path): ), ( "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", ), @@ -377,8 +379,10 @@ def fixture_filled_test_data(template, tmp_path): ), ], ) -def test_validate_data_dict(data_dict, error_message, template, nxdl_root, request): - """Unit test for the data validation routine""" +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", @@ -388,13 +392,25 @@ def test_validate_data_dict(data_dict, error_message, template, nxdl_root, reque "no-child-provided-optional-parent", "int-instead-of-chars", "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 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) 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)