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 <[email protected]>
FAIRmat-NFDI · Dec 20, 2023 · c4d1b9c · c4d1b9c
1 parent 8bd900e
commit c4d1b9c
Show file tree

Hide file tree

Showing 4 changed files with 100 additions and 60 deletions.
diff --git 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,14 +233,15 @@ 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
     -------
     None.
     """
 
     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
@@ -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,19 +650,16 @@ 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
     """
 
     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
@@ -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
@@ -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)