From a1f3698e046b23d88f3818355a5673b20e223adf Mon Sep 17 00:00:00 2001
From: Lukas Pielsticker <50139597+lukaspie@users.noreply.github.com>
Date: Thu, 25 Jul 2024 09:14:55 +0200
Subject: [PATCH] simplify code, make docstrings more elaborate

---
 .../dataconverter/readers/multi/reader.py     | 87 +++++++++++--------
 src/pynxtools/dataconverter/readers/utils.py  |  2 +-
 2 files changed, 53 insertions(+), 36 deletions(-)

diff --git a/src/pynxtools/dataconverter/readers/multi/reader.py b/src/pynxtools/dataconverter/readers/multi/reader.py
index ce0a21e84..15a5ce57d 100644
--- a/src/pynxtools/dataconverter/readers/multi/reader.py
+++ b/src/pynxtools/dataconverter/readers/multi/reader.py
@@ -104,9 +104,17 @@ def __init__(
         self.entry_name = entry_name
 
     def link_callback(self, key: str, value: str) -> Dict[str, Any]:
+        """
+        Modify links to dictionaries with the correct entry name.
+        """
         return {"link": value.replace("/entry/", f"/{self.entry_name}/")}
 
     def identity(self, _: str, value: str) -> str:
+        """
+        Returns the input value unchanged.
+
+        This method serves as an identity function in case no callback is set.
+        """
         return value
 
     def apply_special_key(self, precursor, key, value):
@@ -124,6 +132,13 @@ def resolve_special_keys(
     callbacks: ParseJsonCallbacks,
     suppress_warning: bool = False,
 ) -> None:
+    """
+    Resolves the special keys (denoted by "@") through the callbacks.
+
+    Also takes care of the "!" notation, i.e., removes optional groups
+    if a required sub-element cannot be filled.
+    """
+
     def try_convert(value: str) -> Union[str, float, int, bool]:
         """
         Try to convert the value to float, int or bool.
@@ -140,38 +155,43 @@ def try_convert(value: str) -> Union[str, float, int, bool]:
         return value
 
     def parse_config_value(value: str) -> Tuple[str, Any]:
+        """
+        Separates the prefixes (denoted by "@") from the rest
+        of the value.
+
+        Parameters
+        ----------
+        value : str
+            Config dict value.
+
+        Returns
+        -------
+        Tuple[str, Any]
+            Tuple like (prefix, path).
+
+        """
         # Regex pattern to match @prefix:some_string
         pattern = r"!?(@\w+)(?::(.*))?"
-        match = re.match(pattern, value)
-        if match:
-            prefix, suffix = match.groups()
-            if suffix is None:
-                suffix = ""
-            return (prefix, suffix)
-        else:
+        prefixes = re.findall(pattern, value)
+        if not prefixes:
             return ("", value)
+        return prefixes[0]
 
+    # Handle non-keyword values
     if not isinstance(value, str) or "@" not in str(value):
         if isinstance(value, str):
+            # Handle "!" notation for required fields in optional groups
             value = value.lstrip("!")
         new_entry_dict[key] = value
         return
 
     prefixes: List[Tuple[str, str]] = []
 
-    # Regular expression to check if the string contains a list
-    if re.match(r"^\[.*\]$", value):
-        try:
-            # Safely evaluate the string to a list
-            list_value = ast.literal_eval(value)
-            if isinstance(list_value, list):
-                prefixes = [parse_config_value(v) for v in list_value]
-            else:
-                prefixes = [parse_config_value(value)]
-        except (SyntaxError, ValueError):
-            # If the evaluation fails, treat it as a single string
-            prefixes = [parse_config_value(value)]
-    else:
+    try:
+        # Safely evaluate the string to a list
+        list_value = ast.literal_eval(value)
+        prefixes = [parse_config_value(v) for v in list_value]
+    except (SyntaxError, ValueError):
         prefixes = [parse_config_value(value)]
 
     for prefix, path in prefixes:
@@ -215,10 +235,14 @@ def fill_from_config(
     suppress_warning: bool = False,
 ) -> dict:
     """
-    Parses a json file and returns the data as a dictionary.
+    Parses a config dictionary and returns the data as a dictionary.
     """
 
     def has_missing_main(key: str) -> bool:
+        """
+        Checks if a key starts with a name of a group that has
+        to be removed because a required child is missing.
+        """
         for optional_group in optional_groups_to_remove:
             if key.startswith(optional_group):
                 return True
@@ -393,21 +417,14 @@ def get_processing_order(path: str) -> Tuple[int, Union[str, int]]:
         self.post_process()
 
         if self.config_dict:
-            if "suppress_warning" in kwargs:
-                template.update(
-                    fill_from_config(
-                        self.config_dict,
-                        self.get_entry_names(),
-                        self.callbacks,
-                        kwargs["suppress_warning"],
-                    )
-                )
-            else:
-                template.update(
-                    fill_from_config(
-                        self.config_dict, self.get_entry_names(), self.callbacks
-                    )
+            template.update(
+                fill_from_config(
+                    self.config_dict,
+                    self.get_entry_names(),
+                    self.callbacks,
+                    **{k: v for k, v in kwargs.items() if k == "suppress_warning"},
                 )
+            )
 
         return template
 
diff --git a/src/pynxtools/dataconverter/readers/utils.py b/src/pynxtools/dataconverter/readers/utils.py
index 510923399..9def103c0 100644
--- a/src/pynxtools/dataconverter/readers/utils.py
+++ b/src/pynxtools/dataconverter/readers/utils.py
@@ -39,7 +39,7 @@ class FlattenSettings:
         convert_dict (dict): Dictionary for renaming keys in the flattend dict.
         replace_nested (dict): Dictionary for renaming nested keys.
         parent_key (str, optional):
-            Parent key of the dictionary. Defaults to "/ENTRY[entry]".
+            Parent key of the dictionary. Defaults to "/ENTRY".
         sep (str, optional): Separator for the keys. Defaults to "/".
     """