From 694b696f6c237e95b5d916a8d901ddae0a450412 Mon Sep 17 00:00:00 2001
From: Lukas Pielsticker <50139597+lukaspie@users.noreply.github.com>
Date: Wed, 17 Jan 2024 12:40:24 +0100
Subject: [PATCH 1/5] set logger levels through handler

---
 pynxtools/dataconverter/logger.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/pynxtools/dataconverter/logger.py b/pynxtools/dataconverter/logger.py
index 13a4cf4ea..a0098427c 100644
--- a/pynxtools/dataconverter/logger.py
+++ b/pynxtools/dataconverter/logger.py
@@ -22,4 +22,8 @@
 logger = logging.getLogger("pynxtools")
 
 # Lowest level log allows to other levels erros, crittical, info and debug
+handler = logging.StreamHandler()
+formatter = logging.Formatter("%(message)s")
+handler.setFormatter(formatter)
+logger.addHandler(handler)
 logger.setLevel(logging.DEBUG)

From 2439ee2a66dadaef9abdc6b55a89dd65b93f4a92 Mon Sep 17 00:00:00 2001
From: Sherjeel Shabih <shabihsherjeel@gmail.com>
Date: Wed, 17 Jan 2024 15:45:49 +0100
Subject: [PATCH 2/5] Refactors logging to fit best practices

---
 pynxtools/dataconverter/convert.py | 18 ++++++++----------
 pynxtools/dataconverter/helpers.py | 13 ++++++++-----
 pynxtools/dataconverter/logger.py  | 29 -----------------------------
 pynxtools/dataconverter/writer.py  |  3 +--
 tests/nexus/test_nexus.py          |  8 +++++---
 5 files changed, 22 insertions(+), 49 deletions(-)
 delete mode 100644 pynxtools/dataconverter/logger.py

diff --git a/pynxtools/dataconverter/convert.py b/pynxtools/dataconverter/convert.py
index 767b58769..e6fe1def2 100644
--- a/pynxtools/dataconverter/convert.py
+++ b/pynxtools/dataconverter/convert.py
@@ -33,7 +33,9 @@
 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
+
+logger = logging.getLogger(__name__)
+logger.setLevel(logging.DEBUG)
 
 if sys.version_info >= (3, 10):
     from importlib.metadata import entry_points
@@ -145,7 +147,6 @@ def transfer_data_into_template(
     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.
@@ -183,7 +184,7 @@ def transfer_data_into_template(
 
     bulletpoint = "\n\u2022 "
     logger.info(
-        f"Using {reader} reader reader to convert the given files:"
+        f"Using {reader} reader to convert the given files:"
         f" {bulletpoint.join((' ', *input_file))}"
     )
 
@@ -198,7 +199,7 @@ 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, logger=logger)
+    helpers.validate_data_dict(template, data, nxdl_root)
     return data
 
 
@@ -211,7 +212,6 @@ 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.
@@ -245,7 +245,7 @@ def convert(
     if generate_template:
         template = Template()
         helpers.generate_template_from_nxdl(nxdl_root, template)
-        logger.info(template)
+        print(template)
         return
 
     data = transfer_data_into_template(
@@ -253,7 +253,6 @@ def convert(
         reader=reader,
         nxdl_name=nxdl,
         nxdl_root=nxdl_root,
-        logger=logger,
         **kwargs,
     )
 
@@ -270,9 +269,7 @@ def convert(
                 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))
     Writer(data=data, nxdl_f_path=nxdl_f_path, output_path=output).write()
 
     logger.info(f"The output file generated: {output}.")
@@ -351,6 +348,7 @@ def convert_cli(
     mapping: str,
 ):
     """The CLI entrypoint for the convert function"""
+    logger.addHandler(logging.StreamHandler())
     if params_file:
         try:
             convert(**parse_params_file(params_file))
diff --git a/pynxtools/dataconverter/helpers.py b/pynxtools/dataconverter/helpers.py
index 002c33a61..2b5a9831d 100644
--- a/pynxtools/dataconverter/helpers.py
+++ b/pynxtools/dataconverter/helpers.py
@@ -19,6 +19,7 @@
 
 import json
 import re
+import logging
 import xml.etree.ElementTree as ET
 from datetime import datetime, timezone
 from typing import Any, Callable, List, Optional, Tuple, Union
@@ -28,10 +29,12 @@
 from ase.data import chemical_symbols
 
 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__)
+logger.setLevel(logging.DEBUG)
+
 
 def is_a_lone_group(xml_element) -> bool:
     """Checks whether a given group XML element has no field or attributes mentioned"""
@@ -487,7 +490,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):
     """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 :])
@@ -544,7 +547,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):
     """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."
 
@@ -553,7 +556,7 @@ def validate_data_dict(template, data, nxdl_root: ET.Element, logger=pynx_logger
     nxdl_path_to_elm: dict = {}
 
     # Make sure all required fields exist.
-    ensure_all_required_fields_exist(template, data, nxdl_root, logger)
+    ensure_all_required_fields_exist(template, data, nxdl_root)
     try_undocumented(data, nxdl_root)
 
     for path in data.get_documented().keys():
@@ -650,7 +653,7 @@ def convert_to_hill(atoms_typ):
     return atom_list + list(atoms_typ)
 
 
-def add_default_root_attributes(data, filename, logger=pynx_logger):
+def add_default_root_attributes(data, filename):
     """
     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
deleted file mode 100644
index a0098427c..000000000
--- a/pynxtools/dataconverter/logger.py
+++ /dev/null
@@ -1,29 +0,0 @@
-"""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
-handler = logging.StreamHandler()
-formatter = logging.Formatter("%(message)s")
-handler.setFormatter(formatter)
-logger.addHandler(handler)
-logger.setLevel(logging.DEBUG)
diff --git a/pynxtools/dataconverter/writer.py b/pynxtools/dataconverter/writer.py
index 82ca21198..1a178968f 100644
--- a/pynxtools/dataconverter/writer.py
+++ b/pynxtools/dataconverter/writer.py
@@ -32,8 +32,7 @@
 from pynxtools.dataconverter.exceptions import InvalidDictProvided
 
 logger = logging.getLogger(__name__)  # pylint: disable=C0103
-logger.setLevel(logging.INFO)
-logger.addHandler(logging.StreamHandler(sys.stdout))
+logger.setLevel(logging.DEBUG)
 
 
 def does_path_exist(path, h5py_obj) -> bool:
diff --git a/tests/nexus/test_nexus.py b/tests/nexus/test_nexus.py
index efdb3dfa6..b7daa0505 100644
--- a/tests/nexus/test_nexus.py
+++ b/tests/nexus/test_nexus.py
@@ -25,6 +25,8 @@
 
 from pynxtools.nexus import nexus
 
+logger = logging.getLogger(__name__)
+
 
 def test_get_nexus_classes_units_attributes():
     """Check the correct parsing of a separate list for:
@@ -53,7 +55,7 @@ def test_nexus(tmp_path):
     """
     local_dir = os.path.abspath(os.path.dirname(__file__))
     example_data = os.path.join(local_dir, "../data/nexus/201805_WSe2_arpes.nxs")
-    logger = logging.getLogger(__name__)
+
     logger.setLevel(logging.DEBUG)
     handler = logging.FileHandler(os.path.join(tmp_path, "nexus_test.log"), "w")
     handler.setLevel(logging.DEBUG)
@@ -194,7 +196,7 @@ def test_c_option(tmp_path):
     path_to_ref_files = os.path.join(local_path, "../data/nexus/")
     ref_file = path_to_ref_files + "Ref1_c_option_test.log"
     tmp_file = os.path.join(tmp_path, "c_option_1_test.log")
-    logger = logging.getLogger(__name__)
+
     logger.setLevel(logging.INFO)
     handler = logging.FileHandler(tmp_file, "w")
 
@@ -247,7 +249,7 @@ def test_d_option(tmp_path):
     """
 
     tmp_file = os.path.join(tmp_path, "d_option_1_test.log")
-    logger = logging.getLogger(__name__)
+
     logger.setLevel(logging.DEBUG)
     handler = logging.FileHandler(tmp_file, "w")
 

From 23ab1fb2ca5f5b3e4e1fc988920a6566d237312b Mon Sep 17 00:00:00 2001
From: Sherjeel Shabih <shabihsherjeel@gmail.com>
Date: Wed, 17 Jan 2024 15:48:47 +0100
Subject: [PATCH 3/5] Removes docs for removed arguments in convert function

---
 pynxtools/dataconverter/convert.py | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/pynxtools/dataconverter/convert.py b/pynxtools/dataconverter/convert.py
index e6fe1def2..14975fe7e 100644
--- a/pynxtools/dataconverter/convert.py
+++ b/pynxtools/dataconverter/convert.py
@@ -164,8 +164,6 @@ 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
     -------
@@ -233,8 +231,6 @@ 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
     -------

From c90f3daf1aa97ca9c75bd05671c098e445eb3a75 Mon Sep 17 00:00:00 2001
From: Sherjeel Shabih <shabihsherjeel@gmail.com>
Date: Wed, 17 Jan 2024 17:40:47 +0100
Subject: [PATCH 4/5] Changes loglevels to info

---
 pynxtools/dataconverter/convert.py  | 2 +-
 pynxtools/dataconverter/helpers.py  | 2 +-
 pynxtools/dataconverter/writer.py   | 2 +-
 tests/dataconverter/test_helpers.py | 7 +++----
 4 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/pynxtools/dataconverter/convert.py b/pynxtools/dataconverter/convert.py
index 14975fe7e..c3adaaa8a 100644
--- a/pynxtools/dataconverter/convert.py
+++ b/pynxtools/dataconverter/convert.py
@@ -35,7 +35,7 @@
 from pynxtools.nexus import nexus
 
 logger = logging.getLogger(__name__)
-logger.setLevel(logging.DEBUG)
+logger.setLevel(logging.INFO)
 
 if sys.version_info >= (3, 10):
     from importlib.metadata import entry_points
diff --git a/pynxtools/dataconverter/helpers.py b/pynxtools/dataconverter/helpers.py
index 2b5a9831d..b051747b7 100644
--- a/pynxtools/dataconverter/helpers.py
+++ b/pynxtools/dataconverter/helpers.py
@@ -33,7 +33,7 @@
 from pynxtools.nexus.nexus import NxdlAttributeError
 
 logger = logging.getLogger(__name__)
-logger.setLevel(logging.DEBUG)
+logger.setLevel(logging.INFO)
 
 
 def is_a_lone_group(xml_element) -> bool:
diff --git a/pynxtools/dataconverter/writer.py b/pynxtools/dataconverter/writer.py
index 1a178968f..6532724e0 100644
--- a/pynxtools/dataconverter/writer.py
+++ b/pynxtools/dataconverter/writer.py
@@ -32,7 +32,7 @@
 from pynxtools.dataconverter.exceptions import InvalidDictProvided
 
 logger = logging.getLogger(__name__)  # pylint: disable=C0103
-logger.setLevel(logging.DEBUG)
+logger.setLevel(logging.INFO)
 
 
 def does_path_exist(path, h5py_obj) -> bool:
diff --git a/tests/dataconverter/test_helpers.py b/tests/dataconverter/test_helpers.py
index d368fe955..1c0bd6aca 100644
--- a/tests/dataconverter/test_helpers.py
+++ b/tests/dataconverter/test_helpers.py
@@ -26,7 +26,6 @@
 from setuptools import distutils
 
 from pynxtools.dataconverter import helpers
-from pynxtools.dataconverter.logger import logger as pynx_logger
 from pynxtools.dataconverter.template import Template
 
 
@@ -394,7 +393,7 @@ def test_validate_data_dict(
         "link-dict-instead-of-bool",
         "opt-group-completely-removed",
     ):
-        helpers.validate_data_dict(template, data_dict, nxdl_root, logger=pynx_logger)
+        helpers.validate_data_dict(template, data_dict, nxdl_root)
     # Missing required fields caught by logger with warning
     elif request.node.callspec.id in (
         "empty-required-field",
@@ -406,11 +405,11 @@ def test_validate_data_dict(
         assert "" == caplog.text
         # logger records
         captured_logs = caplog.records
-        helpers.validate_data_dict(template, data_dict, nxdl_root, pynx_logger)
+        helpers.validate_data_dict(template, data_dict, nxdl_root)
         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, pynx_logger)
+            helpers.validate_data_dict(template, data_dict, nxdl_root)
         assert (error_message) == str(execinfo.value)
 
 

From 2c4a804886a3355726014fd4dccfb3e6a6c5d9bf Mon Sep 17 00:00:00 2001
From: Sherjeel Shabih <shabihsherjeel@gmail.com>
Date: Wed, 17 Jan 2024 18:03:24 +0100
Subject: [PATCH 5/5] Fixes generate-template test

---
 tests/dataconverter/test_convert.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/dataconverter/test_convert.py b/tests/dataconverter/test_convert.py
index 0612f47fb..a49cc59a5 100644
--- a/tests/dataconverter/test_convert.py
+++ b/tests/dataconverter/test_convert.py
@@ -120,7 +120,7 @@ def test_cli(caplog, cli_inputs):
     result = runner.invoke(dataconverter.convert_cli, cli_inputs)
     if "--generate-template" in cli_inputs:
         assert result.exit_code == 0
-        assert '"/ENTRY[entry]/NXODD_name/int_value": "None",' in caplog.text
+        assert '"/ENTRY[entry]/NXODD_name/int_value": "None",' in result.stdout
     elif "--input-file" in cli_inputs:
         assert "test_input" in caplog.text
     elif result.exit_code == 2: