From c96c5bed7da446b094e45ef8969dd0ecee6ec85d Mon Sep 17 00:00:00 2001
From: epenet <6771947+epenet@users.noreply.github.com>
Date: Wed, 28 Sep 2022 10:58:04 +0200
Subject: [PATCH] Remove argument validation in Unit Converter (#79107)
* Remove argument validation in Unit Converter
* Use HomeAssistantError
* Adjust tests
* Improve coverage
---
homeassistant/util/unit_conversion.py | 73 +++++++++++++++------------
tests/util/test_distance.py | 5 +-
tests/util/test_pressure.py | 5 +-
tests/util/test_speed.py | 5 +-
tests/util/test_temperature.py | 5 +-
tests/util/test_unit_conversion.py | 7 ++-
tests/util/test_unit_system.py | 11 ++--
tests/util/test_volume.py | 5 +-
8 files changed, 66 insertions(+), 50 deletions(-)
diff --git a/homeassistant/util/unit_conversion.py b/homeassistant/util/unit_conversion.py
index 344aef9028a..45feb083611 100644
--- a/homeassistant/util/unit_conversion.py
+++ b/homeassistant/util/unit_conversion.py
@@ -2,7 +2,6 @@
from __future__ import annotations
from abc import abstractmethod
-from numbers import Number
from homeassistant.const import (
ENERGY_KILO_WATT_HOUR,
@@ -46,6 +45,7 @@ from homeassistant.const import (
VOLUME_LITERS,
VOLUME_MILLILITERS,
)
+from homeassistant.exceptions import HomeAssistantError
# Distance conversion constants
_MM_TO_M = 0.001 # 1 mm = 0.001 m
@@ -78,21 +78,6 @@ class BaseUnitConverter:
NORMALIZED_UNIT: str
VALID_UNITS: set[str]
- @classmethod
- def _check_arguments(cls, value: float, from_unit: str, to_unit: str) -> None:
- """Check that arguments are all valid."""
- if from_unit not in cls.VALID_UNITS:
- raise ValueError(
- UNIT_NOT_RECOGNIZED_TEMPLATE.format(from_unit, cls.UNIT_CLASS)
- )
- if to_unit not in cls.VALID_UNITS:
- raise ValueError(
- UNIT_NOT_RECOGNIZED_TEMPLATE.format(to_unit, cls.UNIT_CLASS)
- )
-
- if not isinstance(value, Number):
- raise TypeError(f"{value} is not of numeric type")
-
@classmethod
@abstractmethod
def convert(cls, value: float, from_unit: str, to_unit: str) -> float:
@@ -107,13 +92,25 @@ class BaseUnitConverterWithUnitConversion(BaseUnitConverter):
@classmethod
def convert(cls, value: float, from_unit: str, to_unit: str) -> float:
"""Convert one unit of measurement to another."""
- cls._check_arguments(value, from_unit, to_unit)
-
if from_unit == to_unit:
return value
- new_value = value / cls.UNIT_CONVERSION[from_unit]
- return new_value * cls.UNIT_CONVERSION[to_unit]
+ try:
+ from_ratio = cls.UNIT_CONVERSION[from_unit]
+ except KeyError as err:
+ raise HomeAssistantError(
+ UNIT_NOT_RECOGNIZED_TEMPLATE.format(from_unit, cls.UNIT_CLASS)
+ ) from err
+
+ try:
+ to_ratio = cls.UNIT_CONVERSION[to_unit]
+ except KeyError as err:
+ raise HomeAssistantError(
+ UNIT_NOT_RECOGNIZED_TEMPLATE.format(to_unit, cls.UNIT_CLASS)
+ ) from err
+
+ new_value = value / from_ratio
+ return new_value * to_ratio
class DistanceConverter(BaseUnitConverterWithUnitConversion):
@@ -247,31 +244,41 @@ class TemperatureConverter(BaseUnitConverter):
cls, value: float, from_unit: str, to_unit: str, *, interval: bool = False
) -> float:
"""Convert a temperature from one unit to another."""
- cls._check_arguments(value, from_unit, to_unit)
-
if from_unit == to_unit:
return value
if from_unit == TEMP_CELSIUS:
if to_unit == TEMP_FAHRENHEIT:
return cls.celsius_to_fahrenheit(value, interval)
- # kelvin
- return cls.celsius_to_kelvin(value, interval)
+ if to_unit == TEMP_KELVIN:
+ return cls.celsius_to_kelvin(value, interval)
+ raise HomeAssistantError(
+ UNIT_NOT_RECOGNIZED_TEMPLATE.format(to_unit, cls.UNIT_CLASS)
+ )
if from_unit == TEMP_FAHRENHEIT:
if to_unit == TEMP_CELSIUS:
return cls.fahrenheit_to_celsius(value, interval)
- # kelvin
- return cls.celsius_to_kelvin(
- cls.fahrenheit_to_celsius(value, interval), interval
+ if to_unit == TEMP_KELVIN:
+ return cls.celsius_to_kelvin(
+ cls.fahrenheit_to_celsius(value, interval), interval
+ )
+ raise HomeAssistantError(
+ UNIT_NOT_RECOGNIZED_TEMPLATE.format(to_unit, cls.UNIT_CLASS)
)
- # from_unit == kelvin
- if to_unit == TEMP_CELSIUS:
- return cls.kelvin_to_celsius(value, interval)
- # fahrenheit
- return cls.celsius_to_fahrenheit(
- cls.kelvin_to_celsius(value, interval), interval
+ if from_unit == TEMP_KELVIN:
+ if to_unit == TEMP_CELSIUS:
+ return cls.kelvin_to_celsius(value, interval)
+ if to_unit == TEMP_FAHRENHEIT:
+ return cls.celsius_to_fahrenheit(
+ cls.kelvin_to_celsius(value, interval), interval
+ )
+ raise HomeAssistantError(
+ UNIT_NOT_RECOGNIZED_TEMPLATE.format(to_unit, cls.UNIT_CLASS)
+ )
+ raise HomeAssistantError(
+ UNIT_NOT_RECOGNIZED_TEMPLATE.format(from_unit, cls.UNIT_CLASS)
)
@classmethod
diff --git a/tests/util/test_distance.py b/tests/util/test_distance.py
index eb001add114..652f95d7bf0 100644
--- a/tests/util/test_distance.py
+++ b/tests/util/test_distance.py
@@ -12,6 +12,7 @@ from homeassistant.const import (
LENGTH_MILLIMETERS,
LENGTH_YARD,
)
+from homeassistant.exceptions import HomeAssistantError
import homeassistant.util.distance as distance_util
INVALID_SYMBOL = "bob"
@@ -32,10 +33,10 @@ def test_convert_same_unit():
def test_convert_invalid_unit():
"""Test exception is thrown for invalid units."""
- with pytest.raises(ValueError):
+ with pytest.raises(HomeAssistantError, match="is not a recognized .* unit"):
distance_util.convert(5, INVALID_SYMBOL, VALID_SYMBOL)
- with pytest.raises(ValueError):
+ with pytest.raises(HomeAssistantError, match="is not a recognized .* unit"):
distance_util.convert(5, VALID_SYMBOL, INVALID_SYMBOL)
diff --git a/tests/util/test_pressure.py b/tests/util/test_pressure.py
index e3b6ed92e4d..13c1a6d1225 100644
--- a/tests/util/test_pressure.py
+++ b/tests/util/test_pressure.py
@@ -11,6 +11,7 @@ from homeassistant.const import (
PRESSURE_PA,
PRESSURE_PSI,
)
+from homeassistant.exceptions import HomeAssistantError
import homeassistant.util.pressure as pressure_util
INVALID_SYMBOL = "bob"
@@ -30,10 +31,10 @@ def test_convert_same_unit():
def test_convert_invalid_unit():
"""Test exception is thrown for invalid units."""
- with pytest.raises(ValueError):
+ with pytest.raises(HomeAssistantError, match="is not a recognized .* unit"):
pressure_util.convert(5, INVALID_SYMBOL, VALID_SYMBOL)
- with pytest.raises(ValueError):
+ with pytest.raises(HomeAssistantError, match="is not a recognized .* unit"):
pressure_util.convert(5, VALID_SYMBOL, INVALID_SYMBOL)
diff --git a/tests/util/test_speed.py b/tests/util/test_speed.py
index 9c7fd070313..f944f0a5d8c 100644
--- a/tests/util/test_speed.py
+++ b/tests/util/test_speed.py
@@ -11,6 +11,7 @@ from homeassistant.const import (
SPEED_MILES_PER_HOUR,
SPEED_MILLIMETERS_PER_DAY,
)
+from homeassistant.exceptions import HomeAssistantError
import homeassistant.util.speed as speed_util
INVALID_SYMBOL = "bob"
@@ -33,10 +34,10 @@ def test_convert_same_unit():
def test_convert_invalid_unit():
"""Test exception is thrown for invalid units."""
- with pytest.raises(ValueError):
+ with pytest.raises(HomeAssistantError, match="is not a recognized .* unit"):
speed_util.convert(5, INVALID_SYMBOL, VALID_SYMBOL)
- with pytest.raises(ValueError):
+ with pytest.raises(HomeAssistantError, match="is not a recognized .* unit"):
speed_util.convert(5, VALID_SYMBOL, INVALID_SYMBOL)
diff --git a/tests/util/test_temperature.py b/tests/util/test_temperature.py
index 71693683fa3..7151d6ed816 100644
--- a/tests/util/test_temperature.py
+++ b/tests/util/test_temperature.py
@@ -2,6 +2,7 @@
import pytest
from homeassistant.const import TEMP_CELSIUS, TEMP_FAHRENHEIT, TEMP_KELVIN
+from homeassistant.exceptions import HomeAssistantError
import homeassistant.util.temperature as temperature_util
INVALID_SYMBOL = "bob"
@@ -34,10 +35,10 @@ def test_convert_same_unit():
def test_convert_invalid_unit():
"""Test exception is thrown for invalid units."""
- with pytest.raises(ValueError):
+ with pytest.raises(HomeAssistantError, match="is not a recognized .* unit"):
temperature_util.convert(5, INVALID_SYMBOL, VALID_SYMBOL)
- with pytest.raises(ValueError):
+ with pytest.raises(HomeAssistantError, match="is not a recognized .* unit"):
temperature_util.convert(5, VALID_SYMBOL, INVALID_SYMBOL)
diff --git a/tests/util/test_unit_conversion.py b/tests/util/test_unit_conversion.py
index fb31624d255..a1835c87f85 100644
--- a/tests/util/test_unit_conversion.py
+++ b/tests/util/test_unit_conversion.py
@@ -41,6 +41,7 @@ from homeassistant.const import (
VOLUME_LITERS,
VOLUME_MILLILITERS,
)
+from homeassistant.exceptions import HomeAssistantError
from homeassistant.util.unit_conversion import (
BaseUnitConverter,
DistanceConverter,
@@ -110,6 +111,8 @@ def test_convert_same_unit(converter: type[BaseUnitConverter], valid_unit: str)
(PressureConverter, PRESSURE_PA),
(SpeedConverter, SPEED_KILOMETERS_PER_HOUR),
(TemperatureConverter, TEMP_CELSIUS),
+ (TemperatureConverter, TEMP_FAHRENHEIT),
+ (TemperatureConverter, TEMP_KELVIN),
(VolumeConverter, VOLUME_LITERS),
],
)
@@ -117,10 +120,10 @@ def test_convert_invalid_unit(
converter: type[BaseUnitConverter], valid_unit: str
) -> None:
"""Test exception is thrown for invalid units."""
- with pytest.raises(ValueError):
+ with pytest.raises(HomeAssistantError, match="is not a recognized .* unit"):
converter.convert(5, INVALID_SYMBOL, valid_unit)
- with pytest.raises(ValueError):
+ with pytest.raises(HomeAssistantError, match="is not a recognized .* unit"):
converter.convert(5, valid_unit, INVALID_SYMBOL)
diff --git a/tests/util/test_unit_system.py b/tests/util/test_unit_system.py
index c4cc94636b1..a284fd10017 100644
--- a/tests/util/test_unit_system.py
+++ b/tests/util/test_unit_system.py
@@ -18,6 +18,7 @@ from homeassistant.const import (
VOLUME_LITERS,
WIND_SPEED,
)
+from homeassistant.exceptions import HomeAssistantError
from homeassistant.util.unit_system import IMPERIAL_SYSTEM, METRIC_SYSTEM, UnitSystem
SYSTEM_NAME = "TEST"
@@ -149,7 +150,7 @@ def test_temperature_same_unit():
def test_temperature_unknown_unit():
"""Test no conversion happens if unknown unit."""
- with pytest.raises(ValueError):
+ with pytest.raises(HomeAssistantError, match="is not a recognized .* unit"):
METRIC_SYSTEM.temperature(5, "abc")
@@ -170,7 +171,7 @@ def test_temperature_to_imperial():
def test_length_unknown_unit():
"""Test length conversion with unknown from unit."""
- with pytest.raises(ValueError):
+ with pytest.raises(HomeAssistantError, match="is not a recognized .* unit"):
METRIC_SYSTEM.length(5, "fr")
@@ -192,7 +193,7 @@ def test_length_to_imperial():
def test_wind_speed_unknown_unit():
"""Test wind_speed conversion with unknown from unit."""
- with pytest.raises(ValueError):
+ with pytest.raises(HomeAssistantError, match="is not a recognized .* unit"):
METRIC_SYSTEM.length(5, "turtles")
@@ -220,7 +221,7 @@ def test_pressure_same_unit():
def test_pressure_unknown_unit():
"""Test no conversion happens if unknown unit."""
- with pytest.raises(ValueError):
+ with pytest.raises(HomeAssistantError, match="is not a recognized .* unit"):
METRIC_SYSTEM.pressure(5, "K")
@@ -252,7 +253,7 @@ def test_accumulated_precipitation_same_unit():
def test_accumulated_precipitation_unknown_unit():
"""Test no conversion happens if unknown unit."""
- with pytest.raises(ValueError):
+ with pytest.raises(HomeAssistantError, match="is not a recognized .* unit"):
METRIC_SYSTEM.accumulated_precipitation(5, "K")
diff --git a/tests/util/test_volume.py b/tests/util/test_volume.py
index 5362f3099fb..c556a3b073d 100644
--- a/tests/util/test_volume.py
+++ b/tests/util/test_volume.py
@@ -10,6 +10,7 @@ from homeassistant.const import (
VOLUME_LITERS,
VOLUME_MILLILITERS,
)
+from homeassistant.exceptions import HomeAssistantError
import homeassistant.util.volume as volume_util
INVALID_SYMBOL = "bob"
@@ -43,10 +44,10 @@ def test_convert_same_unit():
def test_convert_invalid_unit():
"""Test exception is thrown for invalid units."""
- with pytest.raises(ValueError):
+ with pytest.raises(HomeAssistantError, match="is not a recognized .* unit"):
volume_util.convert(5, INVALID_SYMBOL, VALID_SYMBOL)
- with pytest.raises(ValueError):
+ with pytest.raises(HomeAssistantError, match="is not a recognized .* unit"):
volume_util.convert(5, VALID_SYMBOL, INVALID_SYMBOL)
--
GitLab