Merge pull request #251 from andlaus/make_NamedItemList_a_List

Make `NamedItemList` a list when it comes to type checking

examples/somersaultecu.py

@@ -1294,7 +1294,7 @@ class SomersaultSID(IntEnum):
                     sdgs=[],
                 ),
             ]),
-            all_value=None,
+            all_value=True,
         )
 }
 

odxtools/basicstructure.py

@@ -183,7 +183,7 @@ def _validate_coded_message(self, coded_message: bytes) -> None:
             # but it could be that bit_length was mis calculated and not the actual bytes are wrong
             # Could happen with overlapping parameters and parameters with gaps
             warnings.warn(
-                "Verification of coded message {coded_message.hex()} possibly failed: Size may be incorrect.",
+                f"Verification of coded message '{coded_message.hex()}' possibly failed: Size may be incorrect.",
                 OdxWarning,
                 stacklevel=1)
 

odxtools/comparamsubset.py

@@ -57,7 +57,7 @@ def from_et(et_element: ElementTree.Element,
             ComplexComparam.from_et(el, doc_frags)
             for el in et_element.iterfind("COMPLEX-COMPARAMS/COMPLEX-COMPARAM")
         ])
-        if unit_spec_elem := et_element.find("UNIT-SPEC"):
+        if (unit_spec_elem := et_element.find("UNIT-SPEC")) is not None:
             unit_spec = UnitSpec.from_et(unit_spec_elem, doc_frags)
         else:
             unit_spec = None

odxtools/database.py

@@ -12,7 +12,7 @@
 from .diaglayer import DiagLayer
 from .diaglayercontainer import DiagLayerContainer
 from .exceptions import odxraise
-from .nameditemlist import NamedItemList, short_name_as_key
+from .nameditemlist import NamedItemList
 from .odxlink import OdxLinkDatabase
 
 
@@ -85,11 +85,8 @@ def __init__(self,
                     comparam_specs.append(ComparamSpec.from_et(cp_spec, []))
 
         self._diag_layer_containers = NamedItemList(dlcs)
-        self._diag_layer_containers.sort(key=short_name_as_key)
         self._comparam_subsets = NamedItemList(comparam_subsets)
-        self._comparam_subsets.sort(key=short_name_as_key)
         self._comparam_specs = NamedItemList(comparam_specs)
-        self._comparam_specs.sort(key=short_name_as_key)
 
         self.refresh()
 

odxtools/nameditemlist.py

@@ -1,9 +1,8 @@
 # SPDX-License-Identifier: MIT
 import abc
-from collections import OrderedDict
 from keyword import iskeyword
-from typing import (Any, Callable, Collection, Dict, Generic, Iterable, Iterator, List, Optional,
-                    Protocol, Tuple, TypeVar, Union, cast, overload, runtime_checkable)
+from typing import (Any, Collection, Dict, Iterable, List, Optional, Protocol, SupportsIndex, Tuple,
+                    TypeVar, Union, cast, overload, runtime_checkable)
 
 from .exceptions import odxraise
 
@@ -20,7 +19,7 @@ def short_name(self) -> str:
 TNamed = TypeVar("TNamed", bound=OdxNamed)
 
 
-class ItemAttributeList(Generic[T]):
+class ItemAttributeList(List[T]):
     """A list that provides direct access to its items as named attributes.
 
     This is a hybrid between a list and a user-defined object: One can
@@ -35,8 +34,7 @@ class ItemAttributeList(Generic[T]):
     """
 
     def __init__(self, input_list: Optional[Iterable[T]] = None) -> None:
-        self._item_dict: OrderedDict[str, T] = OrderedDict()
-        self._item_list: List[T] = []
+        self._item_dict: Dict[str, T] = {}
 
         if input_list is not None:
             for item in input_list:
@@ -53,6 +51,11 @@ def append(self, item: T) -> None:
 
         \return The name under which item is accessible
         """
+        self._add_attribute_item(item)
+
+        super().append(item)
+
+    def _add_attribute_item(self, item: T) -> None:
         item_name = self._get_item_key(item)
 
         # eliminate conflicts between the name of the new item and
@@ -74,41 +77,58 @@ def append(self, item: T) -> None:
         item_name = tmp
 
         self._item_dict[item_name] = item
-        self._item_list.append(item)
 
-    def sort(self, key: Optional[Callable[[T], str]] = None, reverse: bool = False) -> None:
-        if key is None:
-            self._item_dict = OrderedDict(
-                sorted(self._item_dict.items(), key=lambda x: x[0], reverse=reverse))
-        else:
-            key_fn = cast(Callable[[T], str], key)
-            self._item_dict = OrderedDict(
-                sorted(self._item_dict.items(), key=lambda x: key_fn(x[1]), reverse=reverse))
+    def insert(self, index: SupportsIndex, obj: T) -> None:
+        self._add_attribute_item(obj)
+
+        list.insert(self, index, obj)
+
+    def remove(self, obj: T) -> None:
+        list.remove(self, obj)
+
+        keys = [k for (k, v) in self._item_dict.items() if v == obj]
+        for key in keys:
+            del self._item_dict[key]
 
-        self._item_list = list(self._item_dict.values())
+    def pop(self, index: SupportsIndex = -1) -> T:
+        result = list.pop(self, index)
+        keys = [k for (k, v) in self._item_dict.items() if v == result]
+        for key in keys:
+            del self._item_dict[key]
+        return result
+
+    def extend(self, items: Iterable[T]) -> None:
+        for item in items:
+            self.append(item)
+
+    def clear(self) -> None:
+        super().clear()
+
+        self._item_dict = {}
+
+    def copy(self) -> "ItemAttributeList[T]":
+        result = self.__class__()
+        for item in self:
+            list.append(result, item)
+        result._item_dict = self._item_dict.copy()
+        return result
 
     def keys(self) -> Collection[str]:
         return self._item_dict.keys()
 
     def values(self) -> Collection[T]:
-        return self._item_list
+        return self._item_dict.values()
 
     def items(self) -> Collection[Tuple[str, T]]:
         return self._item_dict.items()
 
-    def __contains__(self, x: T) -> bool:
-        return x in self._item_list
-
-    def __len__(self) -> int:
-        return len(self._item_list)
-
     def __dir__(self) -> Dict[str, Any]:
         result = dict(self.__dict__)
         result.update(self._item_dict)
         return result
 
-    @overload
-    def __getitem__(self, key: int) -> T:
+    @overload  # type: ignore[override]
+    def __getitem__(self, key: SupportsIndex) -> T:
         ...
 
     @overload
@@ -119,11 +139,9 @@ def __getitem__(self, key: str) -> T:
     def __getitem__(self, key: slice) -> List[T]:
         ...
 
-    def __getitem__(self, key: Union[int, str, slice]) -> Union[T, List[T]]:
-        if isinstance(key, int):
-            return self._item_list[key]
-        elif isinstance(key, slice):
-            return self._item_list[key]
+    def __getitem__(self, key: Union[SupportsIndex, str, slice]) -> Union[T, List[T]]:
+        if isinstance(key, (SupportsIndex, slice)):
+            return super().__getitem__(key)
         else:
             return self._item_dict[key]
 
@@ -135,10 +153,9 @@ def __getattr__(self, key: str) -> T:
 
     def get(self, key: Union[int, str], default: Optional[T] = None) -> Optional[T]:
         if isinstance(key, int):
-            if abs(key) < -len(self._item_dict) or key >= len(self._item_dict):
-                return default
-
-            return self._item_list[key]
+            if 0 <= key and key < len(self):
+                return super().__getitem__(key)
+            return default
         else:
             return cast(Optional[T], self._item_dict.get(key, default))
 
@@ -151,42 +168,35 @@ def __eq__(self, other: object) -> bool:
         else:
             return self._item_dict == other._item_dict
 
-    def __iter__(self) -> Iterator[T]:
-        return iter(self._item_list)
-
     def __str__(self) -> str:
-        return f"[{', '.join(self._item_dict.keys())}]"
+        return f"[{', '.join( [self._get_item_key(x) for x in self])}]"
 
     def __repr__(self) -> str:
         return self.__str__()
 
 
-def short_name_as_key(obj: OdxNamed) -> str:
-    """Transform an object's `short_name` attribute into a valid
-    python identifier
+class NamedItemList(ItemAttributeList[T]):
 
-    Although short names are almost identical to valid python
-    identifiers, their first character is allowed to be a number or
-    they may be python keywords. This method prepends an underscore to
-    such short names.
+    def _get_item_key(self, obj: T) -> str:
+        """Transform an object's `short_name` attribute into a valid
+        python identifier
 
-    """
-    if not isinstance(obj, OdxNamed):
-        odxraise()
-    sn = obj.short_name
-    if not isinstance(sn, str):
-        odxraise()
-
-    # make sure that the name of the item in question is not a python
-    # keyword (this would lead to syntax errors) and that does not
-    # start with a digit
-    if sn[0].isdigit() or iskeyword(sn):
-        return f"_{sn}"
-
-    return sn
+        Although short names are almost identical to valid python
+        identifiers, their first character is allowed to be a number or
+        they may be python keywords. This method prepends an underscore to
+        such short names.
 
-
-class NamedItemList(Generic[TNamed], ItemAttributeList[TNamed]):
-
-    def _get_item_key(self, obj: OdxNamed) -> str:
-        return short_name_as_key(obj)
+        """
+        if not isinstance(obj, OdxNamed):
+            odxraise()
+        sn = obj.short_name
+        if not isinstance(sn, str):
+            odxraise()
+
+        # make sure that the name of the item in question is not a python
+        # keyword (this would lead to syntax errors) and that does not
+        # start with a digit
+        if sn[0].isdigit() or iskeyword(sn):
+            return f"_{sn}"
+
+        return sn

tests/test_odxtools.py

@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: MIT
 import unittest
 from dataclasses import dataclass
+from typing import List
 
 import odxtools
 from odxtools.exceptions import OdxError
@@ -97,7 +98,7 @@ class X:
             value: int
 
         foo = NamedItemList([X("hello", 0), X("world", 1)])
-        self.assertEqual(foo.hello, X("hello", 0))  # type: ignore[attr-defined]
+        self.assertEqual(foo.hello, X("hello", 0))
         self.assertEqual(foo[0], X("hello", 0))
         self.assertEqual(foo[1], X("world", 1))
         self.assertEqual(foo[:1], [X("hello", 0)])
@@ -106,15 +107,15 @@ class X:
             foo[2]
         self.assertEqual(foo["hello"], X("hello", 0))
         self.assertEqual(foo["world"], X("world", 1))
-        self.assertEqual(foo.hello, X("hello", 0))  # type: ignore[attr-defined]
-        self.assertEqual(foo.world, X("world", 1))  # type: ignore[attr-defined]
+        self.assertEqual(foo.hello, X("hello", 0))
+        self.assertEqual(foo.world, X("world", 1))
 
         foo.append(X("hello", 2))
         self.assertEqual(foo[2], X("hello", 2))
         self.assertEqual(foo["hello"], X("hello", 0))
         self.assertEqual(foo["hello_2"], X("hello", 2))
-        self.assertEqual(foo.hello, X("hello", 0))  # type: ignore[attr-defined]
-        self.assertEqual(foo.hello_2, X("hello", 2))  # type: ignore[attr-defined]
+        self.assertEqual(foo.hello, X("hello", 0))
+        self.assertEqual(foo.hello_2, X("hello", 2))
 
         # try to append an item that cannot be mapped to a name
         with self.assertRaises(OdxError):
@@ -124,13 +125,13 @@ class X:
         foo.append(X("as", 3))
         self.assertEqual(foo[3], X("as", 3))
         self.assertEqual(foo["_as"], X("as", 3))
-        self.assertEqual(foo._as, X("as", 3))  # type: ignore[attr-defined]
+        self.assertEqual(foo._as, X("as", 3))
 
         # add an object which's name conflicts with a method of the class
         foo.append(X("sort", 4))
         self.assertEqual(foo[4], X("sort", 4))
         self.assertEqual(foo["sort_2"], X("sort", 4))
-        self.assertEqual(foo.sort_2, X("sort", 4))  # type: ignore[attr-defined]
+        self.assertEqual(foo.sort_2, X("sort", 4))
 
         # test the get() function
         self.assertEqual(foo.get(0), X("hello", 0))
@@ -150,6 +151,13 @@ class X:
         self.assertEqual(len(foo.items()), len(foo))
         self.assertEqual(len(foo.values()), len(foo))
 
+        # ensure that mypy accepts NamedItemList objecs where List
+        # objects are expected
+        def bar(x: List[X]) -> None:
+            pass
+
+        bar(foo)
+
 
 class TestNavigation(unittest.TestCase):