[QMI-091] (#93)

fixing mypy issue and improving on unit-tests' coverage and pylint score.

Co-authored-by: Henri Ervasti <[email protected]>

CHANGELOG.md

@@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## \[x.y.z] - Unreleased
 
+### Fixed
+- mypy error on `config_struct.py` by adding extra logic check `and not isinstance(val, type)` on L236.
+- Also made in `config_struct.py` in L186 also tuples to be recognized as "untyped values".
 
 ## [0.45.0] - 2024-07-19
 

qmi/core/config_struct.py

@@ -68,15 +68,14 @@ def initfn(self: Any, **kwargs: Any) -> None:
                 setattr(self, f.name, f.default_factory())
             elif f.init:
                 # No value specified and no default.
-                raise TypeError("{}.__init__() missing required argument {!r}"
-                                .format(cls.__name__, f.name))
+                raise TypeError(f"{cls.__name__}.__init__() missing required argument {f.name!r}")
 
         # Check for unexpected keyword parameters.
         if kwargs:
             for arg_name in kwargs:
                 raise TypeError(
-                    "{}.__init__() got an unexpected keyword argument {!r}"
-                    .format(cls.__name__, arg_name))
+                    f"{cls.__name__}.__init__() got an unexpected keyword argument {arg_name!r}"
+                )
 
     # Invoke dataclass decorator.
     cls = dataclasses.dataclass(init=False)(cls)
@@ -122,7 +121,7 @@ def _inner_config_struct_to_dict(cfg: Any) -> Any:
     elif (cfg is None) or isinstance(cfg, (int, float, bool, str)):
         return cfg
     else:
-        raise TypeError("Unsupported value type: {!r}".format(cfg))
+        raise TypeError(f"Unsupported value type: {cfg!r}")
 
 
 def _dictify_list_value(cfg_list: Union[list, tuple]) -> list:
@@ -135,7 +134,7 @@ def _dictify_dict_value(cfg_dict: dict) -> dict:
     ret: collections.OrderedDict = collections.OrderedDict()
     for (key, value) in cfg_dict.items():
         if not isinstance(key, str):
-            raise TypeError("Unsupported non-string dictionary key: {!r}".format(key))
+            raise TypeError(f"Unsupported non-string dictionary key: {key!r}")
         ret[key] = _inner_config_struct_to_dict(value)
     return ret
 
@@ -157,7 +156,7 @@ def _parse_config_value(val: Any, field_type: Any, path: List[str]) -> Any:
     optional = False
     if repr(field_type).startswith("typing.Union[") or repr(field_type).startswith("typing.Optional["):
         for t in field_type.__args__:
-            if t == type(None):
+            if t == type(None):  # This is intentional! Do not change to 't is None' as it will fail
                 optional = True
             else:
                 field_type = t
@@ -186,11 +185,11 @@ def _parse_config_value(val: Any, field_type: Any, path: List[str]) -> Any:
     elif field_type == Dict:
         field_type = dict
 
-    # Recognize untyped "list" and "dict" values.
-    if (field_type in (list, dict)) and isinstance(val, field_type):
+    # Recognize untyped "list", "tuple" and "dict" values.
+    if (field_type in (list, tuple, dict)) and isinstance(val, field_type):
         return val
 
-    # Recognize untyped "tuple" values.
+    # Recognize untyped "tuple" values that are actually list instances.
     if (field_type is tuple) and isinstance(val, list):
         return tuple(val)
 
@@ -234,25 +233,26 @@ def _parse_config_value(val: Any, field_type: Any, path: List[str]) -> Any:
     if dataclasses.is_dataclass(field_type):
         if isinstance(val, dict):
             return _parse_config_struct(val, field_type, path)
-        elif dataclasses.is_dataclass(val):
+        elif dataclasses.is_dataclass(val) and not isinstance(val, type):
             return _parse_config_struct(dataclasses.asdict(val), field_type, path)
 
     pathstr = ".".join(path)
-    raise QMI_ConfigurationException("Type mismatch in configuration item {}: got {} while expecting {}"
-                                     .format(pathstr, type(val), field_type))
+    raise QMI_ConfigurationException(
+        f"Type mismatch in configuration item {pathstr}: got {type(val)} while expecting {field_type}"
+    )
 
 
 def _parse_config_dict(val: dict, field_type: Any, path: List[str]) -> _StrDict:
     """Parse Config Dict"""
 
     (_, elem_type) = field_type.__args__
-    ret = collections.OrderedDict() # type: _StrDict
+    ret = collections.OrderedDict()  # type: _StrDict
     for (k, elem) in val.items():
         if not isinstance(k, str):
             pathstr = ".".join(path)
             raise QMI_ConfigurationException(
-                "Unsupported non-string dictionary key {!r} in configuration item {}"
-                .format(k, pathstr))
+                f"Unsupported non-string dictionary key {k!r} in configuration item {pathstr}"
+            )
         path.append("[{!r}]".format(k))
         ret[k] = _parse_config_value(elem, elem_type, path)
         path.pop()
@@ -262,7 +262,7 @@ def _parse_config_dict(val: dict, field_type: Any, path: List[str]) -> _StrDict:
 def _parse_config_struct(data: Any, cls: Any, path: List[str]) -> Any:
     """Convert dictionary to dataclass instance."""
 
-    items = collections.OrderedDict() # type: _StrDict
+    items = collections.OrderedDict()  # type: _StrDict
 
     # Walk the list of field definitions.
     for f in dataclasses.fields(cls):
@@ -271,19 +271,19 @@ def _parse_config_struct(data: Any, cls: Any, path: List[str]) -> Any:
             path.append(f.name)
             items[f.name] = _parse_config_value(data[f.name], f.type, path)
             path.pop()
-        elif f.init and (f.default is dataclasses.MISSING) \
-                    and (f.default_factory is dataclasses.MISSING): # type: ignore
+        elif f.init and (f.default is dataclasses.MISSING)\
+                and (f.default_factory is dataclasses.MISSING):  # type: ignore
             # Value not specified and no default.
             path.append(f.name)
             pathstr = ".".join(path)
-            raise QMI_ConfigurationException("Missing value for required configuration item {}".format(pathstr))
+            raise QMI_ConfigurationException(f"Missing value for required configuration item {pathstr}")
 
     # Check for left-over fields.
     for k in data.keys():
         if k not in items:
             path.append(k)
             pathstr = ".".join(path)
-            raise QMI_ConfigurationException("Unknown configuration item {}".format(pathstr))
+            raise QMI_ConfigurationException(f"Unknown configuration item {pathstr}")
 
     # Construct dataclass instance.
     return cls(**items)
@@ -298,13 +298,13 @@ def _check_config_struct_type(cls: Any, path: List[str]) -> None:
     if repr(cls).startswith("typing.Union[") or repr(cls).startswith("typing.Optional["):
         nsub = 0
         for t in cls.__args__:
-            if t == type(None):
+            if t == type(None):  # This is intentional! Do not change to 't is None' as it will fail
                 pass
             elif nsub == 0:
                 cls = t
                 nsub += 1
             else:
-                raise QMI_ConfigurationException("Unsupported Union type in configuration field {}".format(pathstr))
+                raise QMI_ConfigurationException(f"Unsupported Union type in configuration field {pathstr}")
 
     # Recognize scalar types.
     if cls in (int, float, str, bool):
@@ -346,8 +346,9 @@ def _check_config_struct_type(cls: Any, path: List[str]) -> None:
     if type_repr.startswith("typing.Dict["):
         (key_type, elem_type) = cls.__args__
         if key_type is not str:
-            raise QMI_ConfigurationException("Unsupported non-string-key dictionary type in configuration field {}"
-                                             .format(pathstr))
+            raise QMI_ConfigurationException(
+                f"Unsupported non-string-key dictionary type in configuration field {pathstr}"
+            )
         path.append("[]")
         _check_config_struct_type(elem_type, path)
         path.pop()
@@ -364,7 +365,7 @@ def _check_config_struct_type(cls: Any, path: List[str]) -> None:
         return  # accept
 
     # Reject.
-    raise QMI_ConfigurationException("Unsupported data type in configuration field {}".format(pathstr))
+    raise QMI_ConfigurationException(f"Unsupported data type in configuration field {pathstr}")
 
 
 def config_struct_from_dict(data: _StrDict, cls: Type[_T]) -> _T:

tests/core/test_config_struct.py

@@ -3,7 +3,6 @@
 """Unit tests for qmi.core.config_struct."""
 
 import unittest
-
 from dataclasses import field
 from typing import Any, Dict, List, Optional, Tuple, Union
 
@@ -103,6 +102,103 @@ class MyConfig:
         self.assertIsInstance(q2["f"], float)
         self.assertEqual(q2, data2)
 
+    def test_04_class_not_dataclass(self):
+        """The configuration class must be a dataclass"""
+        class MyConfig:
+            i:  int
+            f:  float
+
+        data1 = {"i": 5, "f": 3.0, "3": "str"}
+
+        exp_error = "Configuration class type must be a dataclass"
+        exp_error2 = "Configuration data must be a dataclass instance"
+        with self.assertRaises(TypeError) as type_err:
+            config_struct_from_dict(data1, MyConfig)
+
+        with self.assertRaises(TypeError) as type_err2:
+            config_struct_to_dict(MyConfig)
+
+        self.assertEqual(exp_error, str(type_err.exception))
+        self.assertEqual(exp_error2, str(type_err2.exception))
+
+    def test_05_no_keyword_arguments_allowed(self):
+        """The configstruct doesn't allow keyword arguments"""
+        @configstruct
+        class MyConfig:
+            i:  int
+            f:  float
+
+        exp_error = "config_struct_from_dict() got an unexpected keyword argument 'kwarg'"
+        data1 = {"i": 5, "f": 3.0}
+
+        with self.assertRaises(TypeError) as kw_err:
+            config_struct_from_dict(data1, MyConfig, kwarg=None)
+
+        self.assertEqual(exp_error, str(kw_err.exception))
+
+    def test_06_unsupported_config_type(self):
+        """configstruct dictionary inputs allow only dictionaries with keys as strings."""
+        @configstruct
+        class MyConfig:
+            d:  Dict[str, int]
+
+        dict1 = {"d": {"0": 5, 1: 3.0}}
+        exp_error = "Unsupported non-string dictionary key 1 in configuration item d"
+        with self.assertRaises(QMI_ConfigurationException) as k_err:
+            config_struct_from_dict(dict1, MyConfig)
+
+        self.assertEqual(exp_error, str(k_err.exception))
+
+    def test_07_missing_dataclass_input(self):
+        """Call to get config_struct_from_dict errors when the respective class is 'forgotten' from inputs."""
+        exp_error = "config_struct_from_dict() missing 1 required positional argument: 'cls'"
+        with self.assertRaises(TypeError) as kw_err:
+            config_struct_from_dict({})
+
+        self.assertEqual(exp_error, str(kw_err.exception))
+
+    def test_08_init_missing_argument(self):
+        """The configstruct misses an argument"""
+        class MyConfig:
+            d:  Dict[str, int]
+
+        exp_error = "MyConfig.__init__() missing required argument 'd'"
+        cs = configstruct(MyConfig)
+        with self.assertRaises(TypeError) as miss_err:
+            cs()
+
+        self.assertEqual(exp_error, str(miss_err.exception))
+
+    def test_09_unsupported_value_type_inner(self):
+        """Unsupported value type in _inner_config_struct_to_dict excepts."""
+        @configstruct
+        class MyConfig:
+            i:  int
+
+        exp_error = "Unsupported value type: b'5'"
+
+        data1 = {"i": 5}
+        cfg1 = config_struct_from_dict(data1, MyConfig)
+        cfg1.i = b"5"  # Overwrite the value with an unsupported type
+        with self.assertRaises(TypeError) as type_err:
+            config_struct_to_dict(cfg1)
+
+        self.assertEqual(exp_error, str(type_err.exception))
+
+    def test_10_unsupported_data_type_in_config(self):
+        """Unsupported data type in configstruct excepts."""
+        @configstruct
+        class MyConfig:
+            t:  tuple
+
+        exp_error = "Unsupported data type in configuration field t"
+
+        data1 = {"t": [5,]}
+        with self.assertRaises(QMI_ConfigurationException) as cfg_err:
+            config_struct_from_dict(data1, MyConfig)
+
+        self.assertEqual(exp_error, str(cfg_err.exception))
+
     def test_11_default_values(self):
         """Test a structure with default values for some fields."""
 
@@ -321,6 +417,33 @@ class MyConfig:
 
         self.assertEqual(q1, data1)
 
+    def test_17_typing_types(self):
+        """Test a structure with unspecified Typing field types."""
+
+        @configstruct
+        class MyConfig:
+            vs: List
+            di: Dict
+            t: Tuple
+
+        data1 = {
+            "vs": ["one", "two"],
+            "di": {"one": 1, "thousand": 1000},
+            "t": (5, "qqq"),
+        }
+
+        cfg1 = config_struct_from_dict(data1, MyConfig)
+        self.assertEqual(cfg1, MyConfig(
+            vs=["one", "two"],
+            di={"one": 1, "thousand": 1000},
+            t=(5, "qqq"),
+        ))
+
+        # NOTE: When transforming back to dict, tuple(s) are turned to list(s) in _dictify_list_value.
+        q1 = config_struct_to_dict(cfg1)
+        data1.update({"t": [5, "qqq"]})
+        self.assertEqual(q1, data1)
+
     def test_21_sub_structs(self):
         """Test a structure with sub-structures."""