From 77a4a26a67227e2ad0a53860fbf3f592a0733ca5 Mon Sep 17 00:00:00 2001 From: Amish Date: Thu, 25 Jul 2024 14:01:58 +0200 Subject: [PATCH 01/14] WIP Tweak sensor value setter to validate dtype Still potential to consolidate the sensor-type attributes. Contributes to: NGC-1062. --- src/aiokatcp/sensor.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/aiokatcp/sensor.py b/src/aiokatcp/sensor.py index 60364d0..dfb78df 100644 --- a/src/aiokatcp/sensor.py +++ b/src/aiokatcp/sensor.py @@ -168,6 +168,7 @@ def __init__( self.stype = sensor_type type_info = core.get_type(sensor_type) self.type_name = type_info.name + self._core_type = type_info.type_ self._classic_observers: Set[ClassicObserver[_T]] = set() self._delta_observers: Set[DeltaObserver[_T]] = set() self.name = name @@ -202,6 +203,9 @@ def set_value( ) -> None: """Set the current value of the sensor. + Also check if the incoming value dtype is compatible with the core dtype + of this sensor. + Parameters ---------- value @@ -214,7 +218,18 @@ def set_value( timestamp The time at which the sensor value was determined (seconds). If not given, it defaults to :func:`time.time`. + + Raises + ------ + TypeError + If the incoming `value` type is not compatible with the sensor's + core type. """ + if not issubclass(type(value), self._core_type): + raise TypeError( + f"Value type {type(value)} is not compatible with Sensor type " + f"{self.stype} with core type {self._core_type}" + ) if timestamp is None: timestamp = time.time() if status is None: From f6d3a3d3978dd5529a6ddb20e9a00e0758049112 Mon Sep 17 00:00:00 2001 From: Amish Date: Thu, 8 Aug 2024 11:42:17 +0200 Subject: [PATCH 02/14] Update sensor value setter check As per comments on PR #98, to check the default value and rework logic to account for cases missed in the naive check previously. Contributes to: NGC-1062. --- src/aiokatcp/sensor.py | 42 ++++++++++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/src/aiokatcp/sensor.py b/src/aiokatcp/sensor.py index dfb78df..6ea1f08 100644 --- a/src/aiokatcp/sensor.py +++ b/src/aiokatcp/sensor.py @@ -30,6 +30,7 @@ import enum import functools import inspect +import numbers import time import warnings import weakref @@ -178,7 +179,7 @@ def __init__( if default is None: value: _T = type_info.default(sensor_type) else: - value = default + value = self._check_value_type(default) self._reading = Reading(time.time(), initial_status, value) if auto_strategy is None: self.auto_strategy = SensorSampler.Strategy.AUTO @@ -187,6 +188,31 @@ def __init__( self.auto_strategy_parameters = tuple(auto_strategy_parameters) # TODO: should validate the parameters against the strategy. + def _check_value_type(self, value: _T) -> _T: + """Verify incoming value is compatible with sensor type. + + This also handles the special case of :class:`core.Timestamp` where + it is used with `float` type interchangeably. + + Raises + ------ + TypeError + If the incoming `value` type is not compatible with the sensor's + core type. + """ + + if isinstance(value, self._core_type) and not issubclass(self._core_type, enum.Enum): + return value + elif self.stype is core.Timestamp and isinstance(value, numbers.Real): + return value # type: ignore + elif isinstance(value, self.stype): + return value + else: + raise TypeError( + f"Value type {type(value)} is not compatible with Sensor type " + f"{self.stype} with core type {self._core_type}" + ) + def notify(self, reading: Reading[_T], old_reading: Reading[_T]) -> None: """Notify all observers of changes to this sensor. @@ -203,8 +229,8 @@ def set_value( ) -> None: """Set the current value of the sensor. - Also check if the incoming value dtype is compatible with the core dtype - of this sensor. + Also validate that the incoming value type is compatible with the core + type of this sensor. Parameters ---------- @@ -225,16 +251,12 @@ def set_value( If the incoming `value` type is not compatible with the sensor's core type. """ - if not issubclass(type(value), self._core_type): - raise TypeError( - f"Value type {type(value)} is not compatible with Sensor type " - f"{self.stype} with core type {self._core_type}" - ) + checked_value = self._check_value_type(value) if timestamp is None: timestamp = time.time() if status is None: - status = self.status_func(value) - reading = Reading(timestamp, status, value) + status = self.status_func(checked_value) + reading = Reading(timestamp, status, checked_value) old_reading = self._reading self._reading = reading self.notify(reading, old_reading) From 7732511f7dabb4e412b4275fabe7ebbcaa322544 Mon Sep 17 00:00:00 2001 From: Amish Date: Wed, 14 Aug 2024 09:21:23 +0200 Subject: [PATCH 03/14] Tweak sensor value setter logic for core.Address I'd missed it in the previous update. Contributes to: NGC-1062. --- src/aiokatcp/sensor.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/aiokatcp/sensor.py b/src/aiokatcp/sensor.py index 6ea1f08..9903c04 100644 --- a/src/aiokatcp/sensor.py +++ b/src/aiokatcp/sensor.py @@ -202,10 +202,16 @@ def _check_value_type(self, value: _T) -> _T: """ if isinstance(value, self._core_type) and not issubclass(self._core_type, enum.Enum): + # The more general case of e.g. numpy types not being directly + # comparable with python types. return value elif self.stype is core.Timestamp and isinstance(value, numbers.Real): + # core.Timestamp can also be a float return value # type: ignore + elif type(value) in [bytes, str, core.Address] and self.stype is core.Address: + return core.Address(value) # type: ignore elif isinstance(value, self.stype): + # To ensure specific types of enum.Enums are handled correctly return value else: raise TypeError( From 4f6cedebdea8ba026f65e3c69b1ea3860746cb81 Mon Sep 17 00:00:00 2001 From: Amish Date: Wed, 14 Aug 2024 09:25:06 +0200 Subject: [PATCH 04/14] Add unit test for sensor value setter logic update Contributes to: NGC-1062. --- tests/test_sensor.py | 119 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 119 insertions(+) diff --git a/tests/test_sensor.py b/tests/test_sensor.py index 6e9e2bc..5a4f9f4 100644 --- a/tests/test_sensor.py +++ b/tests/test_sensor.py @@ -30,6 +30,7 @@ """ import asyncio +import enum import gc import unittest import weakref @@ -39,6 +40,7 @@ import pytest +from aiokatcp.core import Address, Timestamp from aiokatcp.sensor import ( AggregateSensor, Reading, @@ -80,6 +82,49 @@ def status_func(value): assert sensor.status == Sensor.Status.NOMINAL +def test_sensor_value_setter(diverse_sensors): + """Check whether the value type verification works.""" + diverse_sensors["test-bool-sensor"].value = True + assert diverse_sensors["test-bool-sensor"] + with pytest.raises(TypeError): + diverse_sensors["test-bool-sensor"].value = "True" + + diverse_sensors["test-int-sensor"].value = 1234 + assert diverse_sensors["test-int-sensor"].value == 1234 + with pytest.raises(TypeError): + diverse_sensors["test-int-sensor"].value = "1234" + + diverse_sensors["test-float-sensor"].value = 1234.5 + assert diverse_sensors["test-float-sensor"].value == 1234.5 + with pytest.raises(TypeError): + diverse_sensors["test-float-sensor"].value = "1234" + + diverse_sensors["test-str-sensor"].value = "foo" + assert diverse_sensors["test-str-sensor"].value == "foo" + with pytest.raises(TypeError): + diverse_sensors["test-str-sensor"].value = 1234 + + diverse_sensors["test-bytes-sensor"].value = b"bar" + assert diverse_sensors["test-bytes-sensor"].value == b"bar" + with pytest.raises(TypeError): + diverse_sensors["test-bytes-sensor"].value = "foo" + + diverse_sensors["test-address-sensor"].value = "1.2.3.4" + assert diverse_sensors["test-address-sensor"].value == Address("1.2.3.4") + with pytest.raises(TypeError): + diverse_sensors["test-address-sensor"].value = 5678 + + diverse_sensors["test-timestamp-sensor"].value = 12345678 + assert diverse_sensors["test-timestamp-sensor"].value == Timestamp(12345678) + with pytest.raises(TypeError): + diverse_sensors["test-timestamp-sensor"].value = "12345678" + + diverse_sensors["test-enum-sensor"].value = MyEnum.ONE + assert diverse_sensors["test-enum-sensor"].value == MyEnum.ONE + with pytest.raises(TypeError): + diverse_sensors["test-enum-sensor"].value = OtherEnum.ABC + + @pytest.fixture def classic_observer(): def classic(sensor, reading): @@ -160,6 +205,80 @@ def alt_sensors(): return [Sensor(float, f"name{i}") for i in range(5)] +class MyEnum(enum.Enum): + ZERO = 0 + ONE = 1 + + +class OtherEnum(enum.Enum): + ABC = 0 + DEF = 1 + + +@pytest.fixture +def diverse_sensors(): + # Another set of sensors with different data types + diverse_ss = SensorSet() + diverse_ss.add( + Sensor( + bool, + "test-bool-sensor", + default=False, + ) + ) + diverse_ss.add( + Sensor( + int, + "test-int-sensor", + default=0, + ) + ) + diverse_ss.add( + Sensor( + float, + "test-float-sensor", + default=0.0, + ) + ) + diverse_ss.add( + Sensor( + str, + "test-str-sensor", + default="zero", + ) + ) + diverse_ss.add( + Sensor( + bytes, + "test-bytes-sensor", + default=b"zero", + ) + ) + diverse_ss.add( + Sensor( + Address, + "test-address-sensor", + default=Address("0.0.0.0"), + ) + ) + diverse_ss.add( + Sensor( + Timestamp, + "test-timestamp-sensor", + default=Timestamp(0.0), + ) + ) + diverse_ss.add( + Sensor( + MyEnum, + "test-enum-sensor", + default=MyEnum.ZERO, + ) + ) + + return diverse_ss + + @pytest.fixture def ss(add_callback, remove_callback, sensors): ss = SensorSet() From d53775324fec370f6ea4f76dee82d0c00baf4b4e Mon Sep 17 00:00:00 2001 From: Amish Date: Wed, 21 Aug 2024 15:45:20 +0200 Subject: [PATCH 05/14] Rework check-sensor-value and unit tests As per @bmerry's comments on #98. Also add numpy to the list of (optional) dependencies, used in the unit test. Contributes to: NGC-1062. --- .pre-commit-config.yaml | 3 +- pyproject.toml | 1 + requirements.in | 1 + requirements.txt | 2 + src/aiokatcp/sensor.py | 34 ++++---- tests/test_sensor.py | 181 ++++++++++++++++------------------------ 6 files changed, 95 insertions(+), 127 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 30f926b..79cfb2e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -41,5 +41,6 @@ repos: 'katcp-codec==0.1.0', 'pytest==8.1.1', 'types-decorator==5.1.1', - 'typing-extensions==4.11.0' + 'typing-extensions==4.11.0', + 'numpy==1.24.4' ] diff --git a/pyproject.toml b/pyproject.toml index 8b6bf8e..0562e50 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,6 +48,7 @@ test = [ "pytest", "pytest-asyncio>=0.23", "pytest-mock", + "numpy", ] doc = [ "sphinx", diff --git a/requirements.in b/requirements.in index 234d89b..4db0d1b 100644 --- a/requirements.in +++ b/requirements.in @@ -9,3 +9,4 @@ pytest-asyncio pytest-cov pytest-mock typing-extensions +numpy diff --git a/requirements.txt b/requirements.txt index 58f62b0..bd45e51 100644 --- a/requirements.txt +++ b/requirements.txt @@ -40,6 +40,8 @@ katcp-codec==0.1.0 # via -r requirements.in nodeenv==1.8.0 # via pre-commit +numpy==1.24.4 + # via -r requirements.in packaging==24.0 # via pytest platformdirs==4.2.0 diff --git a/src/aiokatcp/sensor.py b/src/aiokatcp/sensor.py index 9903c04..12f984d 100644 --- a/src/aiokatcp/sensor.py +++ b/src/aiokatcp/sensor.py @@ -188,11 +188,13 @@ def __init__( self.auto_strategy_parameters = tuple(auto_strategy_parameters) # TODO: should validate the parameters against the strategy. - def _check_value_type(self, value: _T) -> _T: + def _check_value_type(self, value: Any) -> _T: """Verify incoming value is compatible with sensor type. - This also handles the special case of :class:`core.Timestamp` where - it is used with `float` type interchangeably. + If it is compatible, cast it to the expected data type to ensure the + incoming value is in the required format. This also handles the special + case of :class:`core.Timestamp` where it is used with `float` type + interchangeably. Raises ------ @@ -200,19 +202,17 @@ def _check_value_type(self, value: _T) -> _T: If the incoming `value` type is not compatible with the sensor's core type. """ - - if isinstance(value, self._core_type) and not issubclass(self._core_type, enum.Enum): - # The more general case of e.g. numpy types not being directly - # comparable with python types. - return value - elif self.stype is core.Timestamp and isinstance(value, numbers.Real): - # core.Timestamp can also be a float - return value # type: ignore - elif type(value) in [bytes, str, core.Address] and self.stype is core.Address: - return core.Address(value) # type: ignore - elif isinstance(value, self.stype): - # To ensure specific types of enum.Enums are handled correctly + if isinstance(value, self.stype): + # It's an exact match return value + elif isinstance(value, self._core_type) and not issubclass(self._core_type, enum.Enum): + # The more general case of e.g. numpy types not being directly + # comparable with python types. Explicitly cast it into the desired + # type. + return self.stype(value) # type: ignore [call-arg] + elif isinstance(value, numbers.Real) and self.stype is core.Timestamp: + # core.Timestamp can also be an int or float + return self.stype(value) # type: ignore [call-arg] else: raise TypeError( f"Value type {type(value)} is not compatible with Sensor type " @@ -231,7 +231,7 @@ def notify(self, reading: Reading[_T], old_reading: Reading[_T]) -> None: delta_observer(self, reading, old_reading=old_reading) def set_value( - self, value: _T, status: Optional[Status] = None, timestamp: Optional[float] = None + self, value: Any, status: Optional[Status] = None, timestamp: Optional[float] = None ) -> None: """Set the current value of the sensor. @@ -276,7 +276,7 @@ def value(self) -> _T: return self.reading.value @value.setter - def value(self, value: _T) -> None: + def value(self, value: Any) -> None: self.set_value(value) @property diff --git a/tests/test_sensor.py b/tests/test_sensor.py index 5a4f9f4..d7b5b22 100644 --- a/tests/test_sensor.py +++ b/tests/test_sensor.py @@ -34,10 +34,12 @@ import gc import unittest import weakref -from typing import List, Optional +from ipaddress import IPv4Address +from typing import Any, List, Optional, Type, TypeVar from unittest import mock from unittest.mock import create_autospec +import numpy as np import pytest from aiokatcp.core import Address, Timestamp @@ -51,6 +53,8 @@ _weak_callback, ) +_T = TypeVar("_T") + @pytest.mark.parametrize( "status,valid", @@ -82,47 +86,80 @@ def status_func(value): assert sensor.status == Sensor.Status.NOMINAL -def test_sensor_value_setter(diverse_sensors): - """Check whether the value type verification works.""" - diverse_sensors["test-bool-sensor"].value = True - assert diverse_sensors["test-bool-sensor"] - with pytest.raises(TypeError): - diverse_sensors["test-bool-sensor"].value = "True" - - diverse_sensors["test-int-sensor"].value = 1234 - assert diverse_sensors["test-int-sensor"].value == 1234 - with pytest.raises(TypeError): - diverse_sensors["test-int-sensor"].value = "1234" +class MyEnum(enum.Enum): + ZERO = 0 + ONE = 1 - diverse_sensors["test-float-sensor"].value = 1234.5 - assert diverse_sensors["test-float-sensor"].value == 1234.5 - with pytest.raises(TypeError): - diverse_sensors["test-float-sensor"].value = "1234" - diverse_sensors["test-str-sensor"].value = "foo" - assert diverse_sensors["test-str-sensor"].value == "foo" - with pytest.raises(TypeError): - diverse_sensors["test-str-sensor"].value = 1234 +class OtherEnum(enum.Enum): + ABC = 0 + DEF = 1 - diverse_sensors["test-bytes-sensor"].value = b"bar" - assert diverse_sensors["test-bytes-sensor"].value == b"bar" - with pytest.raises(TypeError): - diverse_sensors["test-bytes-sensor"].value = "foo" - diverse_sensors["test-address-sensor"].value = "1.2.3.4" - assert diverse_sensors["test-address-sensor"].value == Address("1.2.3.4") - with pytest.raises(TypeError): - diverse_sensors["test-address-sensor"].value = 5678 +@pytest.mark.parametrize( + "sensor_name,sensor_type,default,compatible_value", + [ + ("bool", bool, False, True), + ("int", int, 0, np.int32(1234)), + ("float", float, 0.0, np.int32(1234678)), + ("float", float, 0.0, np.float32(12345678)), + ("str", str, "zero", "one-two-three-four"), + ("bytes", bytes, b"zero", b"one-two-three-four"), + ("address", Address, Address(IPv4Address("0.0.0.0")), Address(IPv4Address("1.2.3.4"))), + ("timestamp", Timestamp, Timestamp(0.0), 12345678), + ("enum", MyEnum, MyEnum.ZERO, MyEnum.ONE), + ], +) +def test_sensor_value_setter_success( + sensor_name: str, + sensor_type: Type[_T], + default: Any, + compatible_value: Any, +) -> None: + """Check a compatible value against a sensor type. + + This uses a default value to ensure the :class:`Sensor`\'s constructor + also performs the value type check. It also checks compatibility with + numpy scalars. + """ + sensor_under_test = Sensor( + sensor_type, + f"test-{sensor_name}", + default, + ) + sensor_under_test.value = compatible_value + assert sensor_under_test.value == compatible_value - diverse_sensors["test-timestamp-sensor"].value = 12345678 - assert diverse_sensors["test-timestamp-sensor"].value == Timestamp(12345678) - with pytest.raises(TypeError): - diverse_sensors["test-timestamp-sensor"].value = "12345678" - diverse_sensors["test-enum-sensor"].value = MyEnum.ONE - assert diverse_sensors["test-enum-sensor"].value == MyEnum.ONE +@pytest.mark.parametrize( + "sensor_name,sensor_type,bad_value", + [ + ("bool", bool, "True"), + ("int", int, "1234"), + ("float", float, "1234.5"), + ("str", str, {"a": 1}), + ("bytes", bytes, "1"), + ( + "address", + Address, + "0.0.0.0", + ), + ("timestamp", Timestamp, "12345678"), + ("enum", MyEnum, OtherEnum.ABC), + ], +) +def test_sensor_value_setter_failure( + sensor_name: str, + sensor_type: Type[_T], + bad_value: Any, +) -> None: + """Check an incompatible value for a sensor""" + sensor_under_test = Sensor( + sensor_type, + f"test-{sensor_name}", + ) with pytest.raises(TypeError): - diverse_sensors["test-enum-sensor"].value = OtherEnum.ABC + sensor_under_test.value = bad_value @pytest.fixture @@ -205,80 +242,6 @@ def alt_sensors(): return [Sensor(float, f"name{i}") for i in range(5)] -class MyEnum(enum.Enum): - ZERO = 0 - ONE = 1 - - -class OtherEnum(enum.Enum): - ABC = 0 - DEF = 1 - - -@pytest.fixture -def diverse_sensors(): - # Another set of sensors with different data types - diverse_ss = SensorSet() - diverse_ss.add( - Sensor( - bool, - "test-bool-sensor", - default=False, - ) - ) - diverse_ss.add( - Sensor( - int, - "test-int-sensor", - default=0, - ) - ) - diverse_ss.add( - Sensor( - float, - "test-float-sensor", - default=0.0, - ) - ) - diverse_ss.add( - Sensor( - str, - "test-str-sensor", - default="zero", - ) - ) - diverse_ss.add( - Sensor( - bytes, - "test-bytes-sensor", - default=b"zero", - ) - ) - diverse_ss.add( - Sensor( - Address, - "test-address-sensor", - default=Address("0.0.0.0"), - ) - ) - diverse_ss.add( - Sensor( - Timestamp, - "test-timestamp-sensor", - default=Timestamp(0.0), - ) - ) - diverse_ss.add( - Sensor( - MyEnum, - "test-enum-sensor", - default=MyEnum.ZERO, - ) - ) - - return diverse_ss - - @pytest.fixture def ss(add_callback, remove_callback, sensors): ss = SensorSet() From 229b4d0076c1af6859e8faa22481a89061be740e Mon Sep 17 00:00:00 2001 From: Amish Date: Wed, 21 Aug 2024 15:46:36 +0200 Subject: [PATCH 06/14] Add check to core.Address for host arg To ensure it conforms to the ipaddress.{IPv4, IPv6}Address type. Contributes to: NGC-1062. --- src/aiokatcp/core.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/aiokatcp/core.py b/src/aiokatcp/core.py index 8c1d4f4..a30dbac 100644 --- a/src/aiokatcp/core.py +++ b/src/aiokatcp/core.py @@ -79,7 +79,10 @@ class Address: _IPV6_RE = re.compile(r"^\[(?P[^]]+)\](:(?P\d+))?$") def __init__(self, host: _IPAddress, port: Optional[int] = None) -> None: - self._host = host + if isinstance(host, typing.get_args(_IPAddress)): + self._host = host + else: + raise TypeError(f"{host} is not of either {typing.get_args(_IPAddress)}") self._port = port @property From b98ab28f823709ebf74b07e51421e3bbda4e7fc9 Mon Sep 17 00:00:00 2001 From: Amish Date: Tue, 27 Aug 2024 15:36:37 +0200 Subject: [PATCH 07/14] Refine unit test for sensor value setter check As per @bmerry's comments on #98, rework the unit test to be a bit neater and check a few more cases. Also remove the dependency on numpy, and use the builtin fractions module instead. Contributes to: NGC-1062. --- .pre-commit-config.yaml | 1 - requirements.in | 1 - requirements.txt | 2 - src/aiokatcp/sensor.py | 9 +++-- tests/test_sensor.py | 83 ++++++++++++++++++++++------------------- 5 files changed, 49 insertions(+), 47 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 79cfb2e..651b59f 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -42,5 +42,4 @@ repos: 'pytest==8.1.1', 'types-decorator==5.1.1', 'typing-extensions==4.11.0', - 'numpy==1.24.4' ] diff --git a/requirements.in b/requirements.in index 4db0d1b..234d89b 100644 --- a/requirements.in +++ b/requirements.in @@ -9,4 +9,3 @@ pytest-asyncio pytest-cov pytest-mock typing-extensions -numpy diff --git a/requirements.txt b/requirements.txt index bd45e51..58f62b0 100644 --- a/requirements.txt +++ b/requirements.txt @@ -40,8 +40,6 @@ katcp-codec==0.1.0 # via -r requirements.in nodeenv==1.8.0 # via pre-commit -numpy==1.24.4 - # via -r requirements.in packaging==24.0 # via pytest platformdirs==4.2.0 diff --git a/src/aiokatcp/sensor.py b/src/aiokatcp/sensor.py index 12f984d..329b1c8 100644 --- a/src/aiokatcp/sensor.py +++ b/src/aiokatcp/sensor.py @@ -203,12 +203,13 @@ def _check_value_type(self, value: Any) -> _T: core type. """ if isinstance(value, self.stype): - # It's an exact match + # The value is not a special case and can be returned unchanged return value elif isinstance(value, self._core_type) and not issubclass(self._core_type, enum.Enum): - # The more general case of e.g. numpy types not being directly - # comparable with python types. Explicitly cast it into the desired - # type. + # The more general case where the value is not derived from stype + # but is derived from the registered base type. e.g. numpy reals + # don't derive from Python's float, but do derive from numbers.Real. + # Explicitly cast it into the desired type. return self.stype(value) # type: ignore [call-arg] elif isinstance(value, numbers.Real) and self.stype is core.Timestamp: # core.Timestamp can also be an int or float diff --git a/tests/test_sensor.py b/tests/test_sensor.py index d7b5b22..fc3c117 100644 --- a/tests/test_sensor.py +++ b/tests/test_sensor.py @@ -34,12 +34,12 @@ import gc import unittest import weakref +from fractions import Fraction from ipaddress import IPv4Address from typing import Any, List, Optional, Type, TypeVar from unittest import mock from unittest.mock import create_autospec -import numpy as np import pytest from aiokatcp.core import Address, Timestamp @@ -97,69 +97,74 @@ class OtherEnum(enum.Enum): @pytest.mark.parametrize( - "sensor_name,sensor_type,default,compatible_value", + "sensor_type,compatible_value", [ - ("bool", bool, False, True), - ("int", int, 0, np.int32(1234)), - ("float", float, 0.0, np.int32(1234678)), - ("float", float, 0.0, np.float32(12345678)), - ("str", str, "zero", "one-two-three-four"), - ("bytes", bytes, b"zero", b"one-two-three-four"), - ("address", Address, Address(IPv4Address("0.0.0.0")), Address(IPv4Address("1.2.3.4"))), - ("timestamp", Timestamp, Timestamp(0.0), 12345678), - ("enum", MyEnum, MyEnum.ZERO, MyEnum.ONE), + (bool, True), + (int, 1_2_3_4), + (float, Fraction(1234678)), + (float, Fraction(12345678)), + (str, "one-two-three-four"), + (bytes, b"one-two-three-four"), + (Address, Address(IPv4Address("1.2.3.4"))), + (Timestamp, 12345678), + (MyEnum, MyEnum.ONE), ], ) +@pytest.mark.parametrize("initialise_sensor", [True, False]) def test_sensor_value_setter_success( - sensor_name: str, sensor_type: Type[_T], - default: Any, compatible_value: Any, + initialise_sensor: bool, ) -> None: """Check a compatible value against a sensor type. - This uses a default value to ensure the :class:`Sensor`\'s constructor - also performs the value type check. It also checks compatibility with - numpy scalars. + Using `initialise_sensor`, this test checks `compatible_value` either as a + default sensor value, or as a new value to an existing sensor. """ - sensor_under_test = Sensor( + sensor = Sensor( sensor_type, - f"test-{sensor_name}", - default, + "test-sensor", + default=compatible_value if initialise_sensor else None, ) - sensor_under_test.value = compatible_value - assert sensor_under_test.value == compatible_value + if not initialise_sensor: + sensor.value = compatible_value + assert sensor.value == compatible_value + assert isinstance(sensor.value, sensor_type) @pytest.mark.parametrize( - "sensor_name,sensor_type,bad_value", + "sensor_type,bad_value", [ - ("bool", bool, "True"), - ("int", int, "1234"), - ("float", float, "1234.5"), - ("str", str, {"a": 1}), - ("bytes", bytes, "1"), - ( - "address", - Address, - "0.0.0.0", - ), - ("timestamp", Timestamp, "12345678"), - ("enum", MyEnum, OtherEnum.ABC), + (bool, "True"), + (int, "1234"), + (int, 1234.5), + (float, "1234.5"), + (str, {"a": 1}), + (str, b"bytes"), + (bytes, "str"), + (Address, "0.0.0.0"), + (Timestamp, "12345678"), + (MyEnum, OtherEnum.ABC), ], ) def test_sensor_value_setter_failure( - sensor_name: str, sensor_type: Type[_T], bad_value: Any, ) -> None: - """Check an incompatible value for a sensor""" - sensor_under_test = Sensor( + """Check an incompatible value for a sensor. + + Check the `bad_value` is not compatible both as a new value for an existing + sensor and as an initial value for a new sensor. + """ + sensor = Sensor( sensor_type, - f"test-{sensor_name}", + "test-sensor", ) with pytest.raises(TypeError): - sensor_under_test.value = bad_value + sensor.value = bad_value + + with pytest.raises(TypeError): + Sensor(sensor_type, "test-sensor", default=bad_value) @pytest.fixture From d5a2280d86df321b2e15af0d3b22470ab0e2b068 Mon Sep 17 00:00:00 2001 From: Amish Date: Fri, 30 Aug 2024 13:24:28 +0200 Subject: [PATCH 08/14] Remove reference to numpy dependency Missed in a previous commit, apologies. Contributes to: NGC-1062. --- pyproject.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 0562e50..8b6bf8e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,7 +48,6 @@ test = [ "pytest", "pytest-asyncio>=0.23", "pytest-mock", - "numpy", ] doc = [ "sphinx", From 80b7996f6c600a136b2fec7cd10cb18d26bc45a6 Mon Sep 17 00:00:00 2001 From: Amish Date: Fri, 30 Aug 2024 13:25:31 +0200 Subject: [PATCH 09/14] Rework Address init logic As per @bmerry's suggestion on #98. Contributes to: NGC-1062 --- src/aiokatcp/core.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/aiokatcp/core.py b/src/aiokatcp/core.py index a30dbac..d7596fe 100644 --- a/src/aiokatcp/core.py +++ b/src/aiokatcp/core.py @@ -79,10 +79,9 @@ class Address: _IPV6_RE = re.compile(r"^\[(?P[^]]+)\](:(?P\d+))?$") def __init__(self, host: _IPAddress, port: Optional[int] = None) -> None: - if isinstance(host, typing.get_args(_IPAddress)): - self._host = host - else: + if not isinstance(host, typing.get_args(_IPAddress)): raise TypeError(f"{host} is not of either {typing.get_args(_IPAddress)}") + self._host = host self._port = port @property From a4d47d88cb263007607518f602dff6ec907e996d Mon Sep 17 00:00:00 2001 From: Amish Date: Fri, 30 Aug 2024 13:27:30 +0200 Subject: [PATCH 10/14] Refine sensor value setter check To more accurately reflect/describe what it does, as per @bmerry's suggestion on #98. Contributes to: NGC-1062. --- src/aiokatcp/sensor.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/aiokatcp/sensor.py b/src/aiokatcp/sensor.py index 329b1c8..f8c8120 100644 --- a/src/aiokatcp/sensor.py +++ b/src/aiokatcp/sensor.py @@ -179,7 +179,7 @@ def __init__( if default is None: value: _T = type_info.default(sensor_type) else: - value = self._check_value_type(default) + value = self._cast_value_type(default) self._reading = Reading(time.time(), initial_status, value) if auto_strategy is None: self.auto_strategy = SensorSampler.Strategy.AUTO @@ -188,8 +188,8 @@ def __init__( self.auto_strategy_parameters = tuple(auto_strategy_parameters) # TODO: should validate the parameters against the strategy. - def _check_value_type(self, value: Any) -> _T: - """Verify incoming value is compatible with sensor type. + def _cast_value_type(self, value: Any) -> _T: + """Coerce incoming value to the sensor type. If it is compatible, cast it to the expected data type to ensure the incoming value is in the required format. This also handles the special @@ -237,7 +237,8 @@ def set_value( """Set the current value of the sensor. Also validate that the incoming value type is compatible with the core - type of this sensor. + type of this sensor. If compatible, coerce it to an instance of the + :attr:`stype`. Parameters ---------- @@ -258,7 +259,7 @@ def set_value( If the incoming `value` type is not compatible with the sensor's core type. """ - checked_value = self._check_value_type(value) + checked_value = self._cast_value_type(value) if timestamp is None: timestamp = time.time() if status is None: From e40a908d277f3c0887b79089f481e644d0bc7b8c Mon Sep 17 00:00:00 2001 From: Amish Date: Fri, 30 Aug 2024 13:29:44 +0200 Subject: [PATCH 11/14] Tweak sensor value setter unit test params As per @bmerry's suggestion on #98. Contributes to: NGC-1062. --- tests/test_sensor.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_sensor.py b/tests/test_sensor.py index fc3c117..6397265 100644 --- a/tests/test_sensor.py +++ b/tests/test_sensor.py @@ -100,9 +100,9 @@ class OtherEnum(enum.Enum): "sensor_type,compatible_value", [ (bool, True), - (int, 1_2_3_4), - (float, Fraction(1234678)), - (float, Fraction(12345678)), + (int, 1234), + (float, 1234.5), + (float, Fraction(8190 / 64)), (str, "one-two-three-four"), (bytes, b"one-two-three-four"), (Address, Address(IPv4Address("1.2.3.4"))), @@ -138,7 +138,7 @@ def test_sensor_value_setter_success( (bool, "True"), (int, "1234"), (int, 1234.5), - (float, "1234.5"), + (float, 1 + 2j), (str, {"a": 1}), (str, b"bytes"), (bytes, "str"), From af582590fe2e444774f47072029cf7edc8fdef20 Mon Sep 17 00:00:00 2001 From: Amish Date: Fri, 30 Aug 2024 15:15:50 +0200 Subject: [PATCH 12/14] Refine sensor value setter unit test args As per @bmerry's suggestions on #98. Contributes to: NGC-1062. --- tests/test_sensor.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_sensor.py b/tests/test_sensor.py index 6397265..8897fe5 100644 --- a/tests/test_sensor.py +++ b/tests/test_sensor.py @@ -101,8 +101,9 @@ class OtherEnum(enum.Enum): [ (bool, True), (int, 1234), + (float, 1234), (float, 1234.5), - (float, Fraction(8190 / 64)), + (float, Fraction(8190, 64)), (str, "one-two-three-four"), (bytes, b"one-two-three-four"), (Address, Address(IPv4Address("1.2.3.4"))), @@ -112,7 +113,7 @@ class OtherEnum(enum.Enum): ) @pytest.mark.parametrize("initialise_sensor", [True, False]) def test_sensor_value_setter_success( - sensor_type: Type[_T], + sensor_type: Type, compatible_value: Any, initialise_sensor: bool, ) -> None: From ae5ff1fc002ff78f8ef0d6cf0dbbf9d1b1a36435 Mon Sep 17 00:00:00 2001 From: Amish Date: Mon, 2 Sep 2024 14:44:22 +0200 Subject: [PATCH 13/14] Add unit test for core.Address host attribute Specifically to make sure it is of the correct type. Contributes to: NGC-1062. --- tests/test_core.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_core.py b/tests/test_core.py index cd1bdc4..65cc04b 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -131,6 +131,11 @@ def test_parse_bad(self, value) -> None: with pytest.raises(ValueError): Address.parse(value) + @pytest.mark.parametrize("value", ["1.2.3.4"]) + def test_bad_host(self, value) -> None: + with pytest.raises(TypeError): + _ = Address(value) + class TestEncodeDecode: VALUES = [ From 8ac62dbb843e84bb86ad4cb7b48b437e20aa0135 Mon Sep 17 00:00:00 2001 From: Amish Date: Tue, 3 Sep 2024 09:00:11 +0200 Subject: [PATCH 14/14] Refine TestAddress::test_bad_host As per @bmerry's comments on #98. Contributes to: NGC-1062. --- tests/test_core.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_core.py b/tests/test_core.py index 65cc04b..8c0824c 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -131,10 +131,10 @@ def test_parse_bad(self, value) -> None: with pytest.raises(ValueError): Address.parse(value) - @pytest.mark.parametrize("value", ["1.2.3.4"]) + @pytest.mark.parametrize("value", ["127.0.0.1"]) def test_bad_host(self, value) -> None: with pytest.raises(TypeError): - _ = Address(value) + Address(value) class TestEncodeDecode: