From 48d23a9b64fce88e16a9e9554e802ab8c7bae322 Mon Sep 17 00:00:00 2001 From: Andreas Lauser Date: Wed, 20 Dec 2023 09:54:38 +0100 Subject: [PATCH 1/7] BasicStructure: use f-string instead of normal string for warning message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Andreas Lauser Signed-off-by: Katja Köhler --- odxtools/basicstructure.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/odxtools/basicstructure.py b/odxtools/basicstructure.py index 7ebf1147..2807950a 100644 --- a/odxtools/basicstructure.py +++ b/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) From e78a8b209dcdc70d333059dbe2c44175facdda54 Mon Sep 17 00:00:00 2001 From: Andreas Lauser Date: Wed, 20 Dec 2023 09:58:35 +0100 Subject: [PATCH 2/7] move the code of `get_short_name_as_key()` into `NamedItemList` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit there's not much reason to expose this to the general public... Signed-off-by: Andreas Lauser Signed-off-by: Katja Köhler --- odxtools/database.py | 5 +--- odxtools/nameditemlist.py | 48 ++++++++++++++++++--------------------- 2 files changed, 23 insertions(+), 30 deletions(-) diff --git a/odxtools/database.py b/odxtools/database.py index 728cd43d..3cb5d94f 100644 --- a/odxtools/database.py +++ b/odxtools/database.py @@ -11,7 +11,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 @@ -84,11 +84,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() diff --git a/odxtools/nameditemlist.py b/odxtools/nameditemlist.py index 02e8c165..b4688f9b 100644 --- a/odxtools/nameditemlist.py +++ b/odxtools/nameditemlist.py @@ -161,32 +161,28 @@ 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 - - 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. - - """ - 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 - - class NamedItemList(Generic[TNamed], ItemAttributeList[TNamed]): def _get_item_key(self, obj: OdxNamed) -> str: - return short_name_as_key(obj) + """Transform an object's `short_name` attribute into a valid + python identifier + + 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. + + """ + 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 From 2d3bc7a3aa378b9f7edbd17bd13fb82e1e609d33 Mon Sep 17 00:00:00 2001 From: Andreas Lauser Date: Wed, 20 Dec 2023 10:02:20 +0100 Subject: [PATCH 3/7] make NamedItemList a List MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit i.e., whenever a `List` is expected by the type checker, a `NamedItemList` object can now be used as well... Signed-off-by: Andreas Lauser Signed-off-by: Katja Köhler --- odxtools/nameditemlist.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/odxtools/nameditemlist.py b/odxtools/nameditemlist.py index b4688f9b..f878e277 100644 --- a/odxtools/nameditemlist.py +++ b/odxtools/nameditemlist.py @@ -2,8 +2,8 @@ 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, Callable, Collection, Dict, Iterable, Iterator, List, Optional, Protocol, + SupportsIndex, Tuple, TypeVar, Union, cast, overload, runtime_checkable) from .exceptions import odxraise @@ -20,7 +20,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 @@ -76,7 +76,7 @@ def append(self, item: T) -> None: self._item_dict[item_name] = item self._item_list.append(item) - def sort(self, key: Optional[Callable[[T], str]] = None, reverse: bool = False) -> None: + def sort(self, *, key: Any = 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)) @@ -96,7 +96,7 @@ def values(self) -> Collection[T]: def items(self) -> Collection[Tuple[str, T]]: return self._item_dict.items() - def __contains__(self, x: T) -> bool: + def __contains__(self, x: object) -> bool: return x in self._item_list def __len__(self) -> int: @@ -108,7 +108,7 @@ def __dir__(self) -> Dict[str, Any]: return result @overload - def __getitem__(self, key: int) -> T: + def __getitem__(self, key: SupportsIndex) -> T: ... @overload @@ -119,8 +119,8 @@ 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): + def __getitem__(self, key: Union[SupportsIndex, str, slice]) -> Union[T, List[T]]: + if isinstance(key, SupportsIndex): return self._item_list[key] elif isinstance(key, slice): return self._item_list[key] @@ -161,7 +161,7 @@ def __repr__(self) -> str: return self.__str__() -class NamedItemList(Generic[TNamed], ItemAttributeList[TNamed]): +class NamedItemList(ItemAttributeList[TNamed]): def _get_item_key(self, obj: OdxNamed) -> str: """Transform an object's `short_name` attribute into a valid From 52fa82a9e076637d6d26230d5c3b2b0fc9ec7977 Mon Sep 17 00:00:00 2001 From: Andreas Lauser Date: Wed, 20 Dec 2023 13:36:49 +0100 Subject: [PATCH 4/7] NamedItemList: derive it from collections.UserList MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit List is not supposed to be used as a base class. Thanks to [at]kayoub5 for pointing this out. Signed-off-by: Andreas Lauser Signed-off-by: Katja Köhler --- odxtools/database.py | 1 + odxtools/nameditemlist.py | 33 ++++++++++++--------------------- 2 files changed, 13 insertions(+), 21 deletions(-) diff --git a/odxtools/database.py b/odxtools/database.py index 3cb5d94f..896293c4 100644 --- a/odxtools/database.py +++ b/odxtools/database.py @@ -4,6 +4,7 @@ from typing import List, Optional from xml.etree import ElementTree from zipfile import ZipFile + from packaging.version import Version from .comparamspec import ComparamSpec diff --git a/odxtools/nameditemlist.py b/odxtools/nameditemlist.py index f878e277..242829dc 100644 --- a/odxtools/nameditemlist.py +++ b/odxtools/nameditemlist.py @@ -1,9 +1,9 @@ # SPDX-License-Identifier: MIT import abc -from collections import OrderedDict +from collections import OrderedDict, UserList from keyword import iskeyword from typing import (Any, Callable, Collection, Dict, Iterable, Iterator, List, Optional, Protocol, - SupportsIndex, Tuple, TypeVar, Union, cast, overload, runtime_checkable) + SupportsIndex, Tuple, TypeVar, Union, cast, overload, runtime_checkable, Generic) from .exceptions import odxraise @@ -20,7 +20,7 @@ def short_name(self) -> str: TNamed = TypeVar("TNamed", bound=OdxNamed) -class ItemAttributeList(List[T]): +class ItemAttributeList(Generic[T], UserList): """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 @@ -36,7 +36,7 @@ class ItemAttributeList(List[T]): def __init__(self, input_list: Optional[Iterable[T]] = None) -> None: self._item_dict: OrderedDict[str, T] = OrderedDict() - self._item_list: List[T] = [] + self.data: List[T] = [] if input_list is not None: for item in input_list: @@ -74,7 +74,7 @@ def append(self, item: T) -> None: item_name = tmp self._item_dict[item_name] = item - self._item_list.append(item) + self.data.append(item) def sort(self, *, key: Any = None, reverse: bool = False) -> None: if key is None: @@ -85,29 +85,23 @@ def sort(self, *, key: Any = None, reverse: bool = False) -> None: self._item_dict = OrderedDict( sorted(self._item_dict.items(), key=lambda x: key_fn(x[1]), reverse=reverse)) - self._item_list = list(self._item_dict.values()) + self.data = list(self._item_dict.values()) def keys(self) -> Collection[str]: return self._item_dict.keys() def values(self) -> Collection[T]: - return self._item_list + return self.data def items(self) -> Collection[Tuple[str, T]]: return self._item_dict.items() - def __contains__(self, x: object) -> 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 + @overload # type: ignore[override] def __getitem__(self, key: SupportsIndex) -> T: ... @@ -121,9 +115,9 @@ def __getitem__(self, key: slice) -> List[T]: def __getitem__(self, key: Union[SupportsIndex, str, slice]) -> Union[T, List[T]]: if isinstance(key, SupportsIndex): - return self._item_list[key] + return self.data[key] elif isinstance(key, slice): - return self._item_list[key] + return self.data[key] else: return self._item_dict[key] @@ -138,7 +132,7 @@ def get(self, key: Union[int, str], default: Optional[T] = None) -> Optional[T]: if abs(key) < -len(self._item_dict) or key >= len(self._item_dict): return default - return self._item_list[key] + return self.data[key] else: return cast(Optional[T], self._item_dict.get(key, default)) @@ -151,9 +145,6 @@ 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())}]" @@ -161,7 +152,7 @@ def __repr__(self) -> str: return self.__str__() -class NamedItemList(ItemAttributeList[TNamed]): +class NamedItemList(ItemAttributeList[T]): def _get_item_key(self, obj: OdxNamed) -> str: """Transform an object's `short_name` attribute into a valid From abcd8ebe335c50c92ad9fc432b2041a259750261 Mon Sep 17 00:00:00 2001 From: Andreas Lauser Date: Wed, 20 Dec 2023 14:04:12 +0100 Subject: [PATCH 5/7] ComparamSubset: fix deprecation warning appearing with python 3.12 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Checking using `is None` here is a good idea anyway because elements without sub-elements are evaluated to `False`... Signed-off-by: Andreas Lauser Signed-off-by: Katja Köhler --- odxtools/comparamsubset.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/odxtools/comparamsubset.py b/odxtools/comparamsubset.py index bce9cb1b..01ed74c2 100644 --- a/odxtools/comparamsubset.py +++ b/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 From 7864f77d643d51979ec0366bc5d9a00228cac87f Mon Sep 17 00:00:00 2001 From: Andreas Lauser Date: Wed, 20 Dec 2023 14:38:08 +0100 Subject: [PATCH 6/7] improve unit test for `NamedItemList` class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Andreas Lauser Signed-off-by: Katja Köhler --- tests/test_odxtools.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/tests/test_odxtools.py b/tests/test_odxtools.py index c03854b2..ced63fa0 100644 --- a/tests/test_odxtools.py +++ b/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,31 +107,31 @@ 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): - foo.append((0, 3)) # type: ignore[arg-type] + foo.append((0, 3)) # type: ignore[arg-type] # add a keyword identifier 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): From 4f83e272e839d71bb6deef96cd2b49a38ed754e2 Mon Sep 17 00:00:00 2001 From: Andreas Lauser Date: Wed, 20 Dec 2023 14:46:28 +0100 Subject: [PATCH 7/7] derive `NamedItemList` from list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit unfortunately, `mypy` does not seem to treat `UserList` as a subclass of `List`. Thus we need to fall back to deriving from `List`, even if this is strongly discuraged... Signed-off-by: Andreas Lauser Signed-off-by: Katja Köhler --- examples/somersaultecu.py | 2 +- odxtools/nameditemlist.py | 77 +++++++++++++++++++++++++-------------- tests/test_odxtools.py | 2 +- 3 files changed, 52 insertions(+), 29 deletions(-) diff --git a/examples/somersaultecu.py b/examples/somersaultecu.py index 3e1bb4e5..f230d958 100755 --- a/examples/somersaultecu.py +++ b/examples/somersaultecu.py @@ -1274,7 +1274,7 @@ class SomersaultSID(IntEnum): sdgs=[], ), ]), - all_value=None, + all_value=True, ) } diff --git a/odxtools/nameditemlist.py b/odxtools/nameditemlist.py index 242829dc..37a60148 100644 --- a/odxtools/nameditemlist.py +++ b/odxtools/nameditemlist.py @@ -1,9 +1,8 @@ # SPDX-License-Identifier: MIT import abc -from collections import OrderedDict, UserList from keyword import iskeyword -from typing import (Any, Callable, Collection, Dict, Iterable, Iterator, List, Optional, Protocol, - SupportsIndex, Tuple, TypeVar, Union, cast, overload, runtime_checkable, Generic) +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], UserList): +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], UserList): """ def __init__(self, input_list: Optional[Iterable[T]] = None) -> None: - self._item_dict: OrderedDict[str, T] = OrderedDict() - self.data: 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,24 +77,47 @@ def append(self, item: T) -> None: item_name = tmp self._item_dict[item_name] = item - self.data.append(item) - def sort(self, *, key: Any = 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) - self.data = list(self._item_dict.values()) + 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] + + 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.data + return self._item_dict.values() def items(self) -> Collection[Tuple[str, T]]: return self._item_dict.items() @@ -114,10 +140,8 @@ def __getitem__(self, key: slice) -> List[T]: ... def __getitem__(self, key: Union[SupportsIndex, str, slice]) -> Union[T, List[T]]: - if isinstance(key, SupportsIndex): - return self.data[key] - elif isinstance(key, slice): - return self.data[key] + if isinstance(key, (SupportsIndex, slice)): + return super().__getitem__(key) else: return self._item_dict[key] @@ -129,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.data[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)) @@ -146,7 +169,7 @@ def __eq__(self, other: object) -> bool: return self._item_dict == other._item_dict 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__() @@ -154,7 +177,7 @@ def __repr__(self) -> str: class NamedItemList(ItemAttributeList[T]): - def _get_item_key(self, obj: OdxNamed) -> str: + def _get_item_key(self, obj: T) -> str: """Transform an object's `short_name` attribute into a valid python identifier diff --git a/tests/test_odxtools.py b/tests/test_odxtools.py index ced63fa0..4493643a 100644 --- a/tests/test_odxtools.py +++ b/tests/test_odxtools.py @@ -119,7 +119,7 @@ class X: # try to append an item that cannot be mapped to a name with self.assertRaises(OdxError): - foo.append((0, 3)) # type: ignore[arg-type] + foo.append((0, 3)) # type: ignore[arg-type] # add a keyword identifier foo.append(X("as", 3))