From eb6098db13abb7b4dd5d36b95377569285e83f8b Mon Sep 17 00:00:00 2001 From: David Vo Date: Sun, 20 Oct 2024 01:34:06 +1100 Subject: [PATCH 1/5] Explicitly test reading tunables --- tests/test_magicbot_tunable.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_magicbot_tunable.py b/tests/test_magicbot_tunable.py index fad999b..e4268f4 100644 --- a/tests/test_magicbot_tunable.py +++ b/tests/test_magicbot_tunable.py @@ -25,6 +25,7 @@ class Component: topic = nt.getTopic(name) assert topic.getTypeString() == type_str assert topic.genericSubscribe().get().value() == value + assert getattr(component, name) == value for name, value in [ ("rotation", geometry.Rotation2d()), @@ -33,6 +34,7 @@ class Component: assert nt.getTopic(name).getTypeString() == f"struct:{struct_type.__name__}" topic = nt.getStructTopic(name, struct_type) assert topic.subscribe(None).get() == value + assert getattr(component, name) == value for name, struct_type, value in [ ("rotations", geometry.Rotation2d, [geometry.Rotation2d()]), @@ -40,6 +42,7 @@ class Component: assert nt.getTopic(name).getTypeString() == f"struct:{struct_type.__name__}[]" topic = nt.getStructArrayTopic(name, struct_type) assert topic.subscribe([]).get() == value + assert getattr(component, name) == value def test_tunable_errors(): From 7c7748812d1a0f120ab67013cf60b910027b87f5 Mon Sep 17 00:00:00 2001 From: David Vo Date: Sun, 20 Oct 2024 01:34:38 +1100 Subject: [PATCH 2/5] tunable: Allow empty default lists when type-hinted --- magicbot/magic_tunable.py | 45 +++++++++++++++++++++++++++++++--- tests/test_magicbot_tunable.py | 40 +++++++++++++++++++++++++++++- 2 files changed, 80 insertions(+), 5 deletions(-) diff --git a/magicbot/magic_tunable.py b/magicbot/magic_tunable.py index d8e3570..617bfd9 100644 --- a/magicbot/magic_tunable.py +++ b/magicbot/magic_tunable.py @@ -80,6 +80,7 @@ def execute(self): "_ntsubtable", "_ntwritedefault", # "__doc__", + "__orig_class__", "_topic_type", "_nt", ) @@ -100,13 +101,47 @@ def __init__( self._ntwritedefault = writeDefault # self.__doc__ = doc - self._topic_type = _get_topic_type_for_value(self._ntdefault) - if self._topic_type is None: - checked_type: type = type(self._ntdefault) + # Defer checks for empty sequences to check type hints. + # Report errors here when we can so the error points to the tunable line. + if default or not isinstance(default, collections.abc.Sequence): + self._topic_type = _get_topic_type_for_value(default) + if self._topic_type is None: + checked_type: type = type(default) + raise TypeError( + f"tunable is not publishable to NetworkTables, type: {checked_type.__name__}" + ) + + def __set_name__(self, owner: type, name: str) -> None: + type_hint: Optional[type] = None + # __orig_class__ is set after __init__, check it here. + orig_class = getattr(self, "__orig_class__", None) + if orig_class is not None: + # Accept field = tunable[Sequence[int]]([]) + type_hint = typing.get_args(orig_class)[0] + else: + type_hint = typing.get_type_hints(owner).get(name) + origin = typing.get_origin(type_hint) + if origin is typing.ClassVar: + # Accept field: ClassVar[tunable[Sequence[int]]] = tunable([]) + type_hint = typing.get_args(type_hint)[0] + origin = typing.get_origin(type_hint) + if origin is tunable: + # Accept field: tunable[Sequence[int]] = tunable([]) + type_hint = typing.get_args(type_hint)[0] + + if type_hint is not None: + topic_type = _get_topic_type(type_hint) + else: + topic_type = _get_topic_type_for_value(self._ntdefault) + + if topic_type is None: + checked_type: type = type_hint or type(self._ntdefault) raise TypeError( f"tunable is not publishable to NetworkTables, type: {checked_type.__name__}" ) + self._topic_type = topic_type + @overload def __get__(self, instance: None, owner=None) -> "tunable[V]": ... @@ -218,7 +253,7 @@ class MyComponent: navx: ... @feedback - def get_angle(self): + def get_angle(self) -> float: return self.navx.getYaw() class MyRobot(magicbot.MagicRobot): @@ -297,6 +332,8 @@ def _get_topic_type( if hasattr(inner_type, "WPIStruct"): return lambda topic: ntcore.StructArrayTopic(topic, inner_type) + return None + def collect_feedbacks(component, cname: str, prefix: Optional[str] = "components"): """ diff --git a/tests/test_magicbot_tunable.py b/tests/test_magicbot_tunable.py index e4268f4..c47eb55 100644 --- a/tests/test_magicbot_tunable.py +++ b/tests/test_magicbot_tunable.py @@ -1,3 +1,5 @@ +from typing import ClassVar, List, Sequence + import ntcore import pytest from wpimath import geometry @@ -53,7 +55,43 @@ class Component: def test_tunable_errors_with_empty_sequence(): - with pytest.raises(ValueError): + with pytest.raises(RuntimeError): class Component: empty = tunable([]) + + +def test_type_hinted_empty_sequences() -> None: + class Component: + generic_seq = tunable[Sequence[int]](()) + class_var_seq: ClassVar[tunable[Sequence[int]]] = tunable(()) + inst_seq: Sequence[int] = tunable(()) + + generic_typing_list = tunable[List[int]]([]) + class_var_typing_list: ClassVar[tunable[List[int]]] = tunable([]) + inst_typing_list: List[int] = tunable([]) + + generic_list = tunable[list[int]]([]) + class_var_list: ClassVar[tunable[list[int]]] = tunable([]) + inst_list: list[int] = tunable([]) + + component = Component() + setup_tunables(component, "test_type_hinted_sequences") + NetworkTables = ntcore.NetworkTableInstance.getDefault() + nt = NetworkTables.getTable("/components/test_type_hinted_sequences") + + for name in [ + "generic_seq", + "class_var_seq", + "inst_seq", + "generic_typing_list", + "class_var_typing_list", + "inst_typing_list", + "generic_list", + "class_var_list", + "inst_list", + ]: + assert nt.getTopic(name).getTypeString() == "int[]" + entry = nt.getEntry(name) + assert entry.getIntegerArray(None) == [] + assert getattr(component, name) == [] From ba0e8fdd7f3f3ef68c2a0268451aee6d44b8568d Mon Sep 17 00:00:00 2001 From: David Vo Date: Sun, 20 Oct 2024 01:39:14 +1100 Subject: [PATCH 3/5] tunable: Make mypy happier with type narrowing --- magicbot/magic_tunable.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/magicbot/magic_tunable.py b/magicbot/magic_tunable.py index 617bfd9..091e2d8 100644 --- a/magicbot/magic_tunable.py +++ b/magicbot/magic_tunable.py @@ -104,12 +104,13 @@ def __init__( # Defer checks for empty sequences to check type hints. # Report errors here when we can so the error points to the tunable line. if default or not isinstance(default, collections.abc.Sequence): - self._topic_type = _get_topic_type_for_value(default) - if self._topic_type is None: + topic_type = _get_topic_type_for_value(default) + if topic_type is None: checked_type: type = type(default) raise TypeError( f"tunable is not publishable to NetworkTables, type: {checked_type.__name__}" ) + self._topic_type = topic_type def __set_name__(self, owner: type, name: str) -> None: type_hint: Optional[type] = None From 285c7096e86e818c6be4efc6136afe7c5973d992 Mon Sep 17 00:00:00 2001 From: David Vo Date: Sun, 20 Oct 2024 01:59:59 +1100 Subject: [PATCH 4/5] Disable PEP 585 test --- tests/test_magicbot_tunable.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/test_magicbot_tunable.py b/tests/test_magicbot_tunable.py index c47eb55..70f6ac7 100644 --- a/tests/test_magicbot_tunable.py +++ b/tests/test_magicbot_tunable.py @@ -71,9 +71,10 @@ class Component: class_var_typing_list: ClassVar[tunable[List[int]]] = tunable([]) inst_typing_list: List[int] = tunable([]) - generic_list = tunable[list[int]]([]) - class_var_list: ClassVar[tunable[list[int]]] = tunable([]) - inst_list: list[int] = tunable([]) + # TODO(davo): re-enable after py3.8 is dropped + # generic_list = tunable[list[int]]([]) + # class_var_list: ClassVar[tunable[list[int]]] = tunable([]) + # inst_list: list[int] = tunable([]) component = Component() setup_tunables(component, "test_type_hinted_sequences") @@ -87,9 +88,9 @@ class Component: "generic_typing_list", "class_var_typing_list", "inst_typing_list", - "generic_list", - "class_var_list", - "inst_list", + # "generic_list", + # "class_var_list", + # "inst_list", ]: assert nt.getTopic(name).getTypeString() == "int[]" entry = nt.getEntry(name) From 20ea22edb509054a188fe74b144cedf7636d9c50 Mon Sep 17 00:00:00 2001 From: David Vo Date: Sun, 20 Oct 2024 02:03:21 +1100 Subject: [PATCH 5/5] tunable: Fix empty sequence test on py3.12 --- tests/test_magicbot_tunable.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_magicbot_tunable.py b/tests/test_magicbot_tunable.py index 70f6ac7..a90a13b 100644 --- a/tests/test_magicbot_tunable.py +++ b/tests/test_magicbot_tunable.py @@ -55,7 +55,7 @@ class Component: def test_tunable_errors_with_empty_sequence(): - with pytest.raises(RuntimeError): + with pytest.raises((RuntimeError, ValueError)): class Component: empty = tunable([])