Rename IMPERIAL_SYSTEM to US_CUSTOMARY_SYSTEM (#80253)

* Rename IMPERIAL_SYSTEM

* Deprecate is_metric property and adjust tests

* Adjust unit_system config validation

* Add yaml tests

* Add tests for private name

* Fix incorrect rebase

* Adjust docstring

* Add store migration

* Update unit_system.py

* Minimise test tweaks

* Fix tests

* Add conversion to migration

* Rename new key and adjust tests

* Adjust websocket_detect_config

* Move original_unit_system tracking to subclass

homeassistant/components/config/core.py

@@ -94,7 +94,7 @@ async def websocket_detect_config(
         info["unit_system"] = unit_system._CONF_UNIT_SYSTEM_METRIC
     else:
         # pylint: disable-next=protected-access
-        info["unit_system"] = unit_system._CONF_UNIT_SYSTEM_IMPERIAL
+        info["unit_system"] = unit_system._CONF_UNIT_SYSTEM_US_CUSTOMARY
 
     if location_info.latitude:
         info["latitude"] = location_info.latitude

homeassistant/core.py

@@ -81,7 +81,13 @@ from .util.async_ import (
 )
 from .util.read_only_dict import ReadOnlyDict
 from .util.timeout import TimeoutManager
-from .util.unit_system import METRIC_SYSTEM, UnitSystem, get_unit_system
+from .util.unit_system import (
+    _CONF_UNIT_SYSTEM_IMPERIAL,
+    _CONF_UNIT_SYSTEM_US_CUSTOMARY,
+    METRIC_SYSTEM,
+    UnitSystem,
+    get_unit_system,
+)
 
 # Typing imports that create a circular dependency
 if TYPE_CHECKING:
@@ -107,6 +113,7 @@ CALLBACK_TYPE = Callable[[], None]  # pylint: disable=invalid-name
 
 CORE_STORAGE_KEY = "core.config"
 CORE_STORAGE_VERSION = 1
+CORE_STORAGE_MINOR_VERSION = 2
 
 DOMAIN = "homeassistant"
 
@@ -1986,7 +1993,7 @@ class Config:
             latitude=data.get("latitude"),
             longitude=data.get("longitude"),
             elevation=data.get("elevation"),
-            unit_system=data.get("unit_system"),
+            unit_system=data.get("unit_system_v2"),
             location_name=data.get("location_name"),
             time_zone=data.get("time_zone"),
             external_url=data.get("external_url", _UNDEF),
@@ -2002,7 +2009,7 @@ class Config:
             "elevation": self.elevation,
             # We don't want any integrations to use the name of the unit system
             # so we are using the private attribute here
-            "unit_system": self.units._name,  # pylint: disable=protected-access
+            "unit_system_v2": self.units._name,  # pylint: disable=protected-access
             "location_name": self.location_name,
             "time_zone": self.time_zone,
             "external_url": self.external_url,
@@ -2027,4 +2034,30 @@ class Config:
                 CORE_STORAGE_KEY,
                 private=True,
                 atomic_writes=True,
+                minor_version=CORE_STORAGE_MINOR_VERSION,
             )
+            self._original_unit_system: str | None = None  # from old store 1.1
+
+        async def _async_migrate_func(
+            self,
+            old_major_version: int,
+            old_minor_version: int,
+            old_data: dict[str, Any],
+        ) -> dict[str, Any]:
+            """Migrate to the new version."""
+            data = old_data
+            if old_major_version == 1 and old_minor_version < 2:
+                # In 1.2, we remove support for "imperial", replaced by "us_customary"
+                # Using a new key to allow rollback
+                self._original_unit_system = data.get("unit_system")
+                data["unit_system_v2"] = self._original_unit_system
+                if data["unit_system_v2"] == _CONF_UNIT_SYSTEM_IMPERIAL:
+                    data["unit_system_v2"] = _CONF_UNIT_SYSTEM_US_CUSTOMARY
+            if old_major_version > 1:
+                raise NotImplementedError
+            return data
+
+        async def async_save(self, data: dict[str, Any]) -> None:
+            if self._original_unit_system:
+                data["unit_system"] = self._original_unit_system
+            return await super().async_save(data)

homeassistant/util/unit_system.py

@@ -44,6 +44,7 @@ from .unit_conversion import (
 
 _CONF_UNIT_SYSTEM_IMPERIAL: Final = "imperial"
 _CONF_UNIT_SYSTEM_METRIC: Final = "metric"
+_CONF_UNIT_SYSTEM_US_CUSTOMARY: Final = "us_customary"
 
 LENGTH_UNITS = DistanceConverter.VALID_UNITS
 
@@ -130,6 +131,9 @@ class UnitSystem:
             "Please adjust to use instance check instead.",
             error_if_core=False,
         )
+        if self is IMPERIAL_SYSTEM:
+            # kept for compatibility reasons, with associated warning above
+            return _CONF_UNIT_SYSTEM_IMPERIAL
         return self._name
 
     @property
@@ -213,15 +217,26 @@ class UnitSystem:
 
 def get_unit_system(key: str) -> UnitSystem:
     """Get unit system based on key."""
-    if key == _CONF_UNIT_SYSTEM_IMPERIAL:
+    if key == _CONF_UNIT_SYSTEM_US_CUSTOMARY:
-        return IMPERIAL_SYSTEM
+        return US_CUSTOMARY_SYSTEM
     if key == _CONF_UNIT_SYSTEM_METRIC:
         return METRIC_SYSTEM
     raise ValueError(f"`{key}` is not a valid unit system key")
 
 
+def _deprecated_unit_system(value: str) -> str:
+    """Convert deprecated unit system."""
+
+    if value == _CONF_UNIT_SYSTEM_IMPERIAL:
+        # need to add warning in 2023.1
+        return _CONF_UNIT_SYSTEM_US_CUSTOMARY
+    return value
+
+
 validate_unit_system = vol.All(
-    vol.Lower, vol.Any(_CONF_UNIT_SYSTEM_METRIC, _CONF_UNIT_SYSTEM_IMPERIAL)
+    vol.Lower,
+    _deprecated_unit_system,
+    vol.Any(_CONF_UNIT_SYSTEM_METRIC, _CONF_UNIT_SYSTEM_US_CUSTOMARY),
 )
 
 METRIC_SYSTEM = UnitSystem(
@@ -235,8 +250,8 @@ METRIC_SYSTEM = UnitSystem(
     LENGTH_MILLIMETERS,
 )
 
-IMPERIAL_SYSTEM = UnitSystem(
+US_CUSTOMARY_SYSTEM = UnitSystem(
-    _CONF_UNIT_SYSTEM_IMPERIAL,
+    _CONF_UNIT_SYSTEM_US_CUSTOMARY,
     TEMP_FAHRENHEIT,
     LENGTH_MILES,
     SPEED_MILES_PER_HOUR,
@@ -245,3 +260,6 @@ IMPERIAL_SYSTEM = UnitSystem(
     PRESSURE_PSI,
     LENGTH_INCHES,
 )
+
+IMPERIAL_SYSTEM = US_CUSTOMARY_SYSTEM
+"""IMPERIAL_SYSTEM is deprecated. Please use US_CUSTOMARY_SYSTEM instead."""

tests/test_config.py

@@ -27,11 +27,17 @@ from homeassistant.const import (
     CONF_UNIT_SYSTEM_METRIC,
     __version__,
 )
-from homeassistant.core import ConfigSource, HomeAssistantError
+from homeassistant.core import ConfigSource, HomeAssistant, HomeAssistantError
 from homeassistant.helpers import config_validation as cv
 import homeassistant.helpers.check_config as check_config
 from homeassistant.helpers.entity import Entity
 from homeassistant.loader import async_get_integration
+from homeassistant.util.unit_system import (
+    _CONF_UNIT_SYSTEM_US_CUSTOMARY,
+    METRIC_SYSTEM,
+    US_CUSTOMARY_SYSTEM,
+    UnitSystem,
+)
 from homeassistant.util.yaml import SECRET_YAML
 
 from tests.common import get_test_config_dir, patch_yaml_files
@@ -439,7 +445,7 @@ async def test_loading_configuration_from_storage_with_yaml_only(hass, hass_stor
     assert hass.config.config_source is ConfigSource.STORAGE
 
 
-async def test_updating_configuration(hass, hass_storage):
+async def test_igration_and_updating_configuration(hass, hass_storage):
     """Test updating configuration stores the new configuration."""
     core_data = {
         "data": {
@@ -448,7 +454,7 @@ async def test_updating_configuration(hass, hass_storage):
             "location_name": "Home",
             "longitude": 13,
             "time_zone": "Europe/Copenhagen",
-            "unit_system": "metric",
+            "unit_system": "imperial",
             "external_url": "https://www.example.com",
             "internal_url": "http://example.local",
             "currency": "BTC",
@@ -463,10 +469,14 @@ async def test_updating_configuration(hass, hass_storage):
     )
     await hass.config.async_update(latitude=50, currency="USD")
 
-    new_core_data = copy.deepcopy(core_data)
+    expected_new_core_data = copy.deepcopy(core_data)
-    new_core_data["data"]["latitude"] = 50
+    # From async_update above
-    new_core_data["data"]["currency"] = "USD"
+    expected_new_core_data["data"]["latitude"] = 50
-    assert hass_storage["core.config"] == new_core_data
+    expected_new_core_data["data"]["currency"] = "USD"
+    # 1.1 -> 1.2 store migration with migrated unit system
+    expected_new_core_data["data"]["unit_system_v2"] = "us_customary"
+    expected_new_core_data["minor_version"] = 2
+    assert hass_storage["core.config"] == expected_new_core_data
     assert hass.config.latitude == 50
     assert hass.config.currency == "USD"
 
@@ -593,6 +603,35 @@ async def test_loading_configuration_from_packages(hass):
         )
 
 
+@pytest.mark.parametrize(
+    "unit_system_name, expected_unit_system",
+    [
+        (CONF_UNIT_SYSTEM_METRIC, METRIC_SYSTEM),
+        (CONF_UNIT_SYSTEM_IMPERIAL, US_CUSTOMARY_SYSTEM),
+        (_CONF_UNIT_SYSTEM_US_CUSTOMARY, US_CUSTOMARY_SYSTEM),
+    ],
+)
+async def test_loading_configuration_unit_system(
+    hass: HomeAssistant, unit_system_name: str, expected_unit_system: UnitSystem
+) -> None:
+    """Test backward compatibility when loading core config."""
+    await config_util.async_process_ha_core_config(
+        hass,
+        {
+            "latitude": 60,
+            "longitude": 50,
+            "elevation": 25,
+            "name": "Huis",
+            "unit_system": unit_system_name,
+            "time_zone": "America/New_York",
+            "external_url": "https://www.example.com",
+            "internal_url": "http://example.local",
+        },
+    )
+
+    assert hass.config.units is expected_unit_system
+
+
 @patch("homeassistant.helpers.check_config.async_check_ha_config_file")
 async def test_check_ha_config_file_correct(mock_check, hass):
     """Check that restart propagates to stop."""

tests/util/test_unit_system.py

@@ -22,8 +22,10 @@ from homeassistant.exceptions import HomeAssistantError
 from homeassistant.util.unit_system import (
     _CONF_UNIT_SYSTEM_IMPERIAL,
     _CONF_UNIT_SYSTEM_METRIC,
+    _CONF_UNIT_SYSTEM_US_CUSTOMARY,
     IMPERIAL_SYSTEM,
     METRIC_SYSTEM,
+    US_CUSTOMARY_SYSTEM,
     UnitSystem,
     get_unit_system,
 )
@@ -320,17 +322,26 @@ def test_is_metric(
 
 
 @pytest.mark.parametrize(
-    "unit_system, expected_name",
+    "unit_system, expected_name, expected_private_name",
     [
-        (METRIC_SYSTEM, _CONF_UNIT_SYSTEM_METRIC),
+        (METRIC_SYSTEM, _CONF_UNIT_SYSTEM_METRIC, _CONF_UNIT_SYSTEM_METRIC),
-        (IMPERIAL_SYSTEM, _CONF_UNIT_SYSTEM_IMPERIAL),
+        (IMPERIAL_SYSTEM, _CONF_UNIT_SYSTEM_IMPERIAL, _CONF_UNIT_SYSTEM_US_CUSTOMARY),
+        (
+            US_CUSTOMARY_SYSTEM,
+            _CONF_UNIT_SYSTEM_IMPERIAL,
+            _CONF_UNIT_SYSTEM_US_CUSTOMARY,
+        ),
     ],
 )
 def test_deprecated_name(
-    caplog: pytest.LogCaptureFixture, unit_system: UnitSystem, expected_name: str
+    caplog: pytest.LogCaptureFixture,
+    unit_system: UnitSystem,
+    expected_name: str,
+    expected_private_name: str,
 ) -> None:
     """Test the name is deprecated."""
     assert unit_system.name == expected_name
+    assert unit_system._name == expected_private_name
     assert (
         "Detected code that accesses the `name` property of the unit system."
         in caplog.text
@@ -341,7 +352,7 @@ def test_deprecated_name(
     "key, expected_system",
     [
         (_CONF_UNIT_SYSTEM_METRIC, METRIC_SYSTEM),
-        (_CONF_UNIT_SYSTEM_IMPERIAL, IMPERIAL_SYSTEM),
+        (_CONF_UNIT_SYSTEM_US_CUSTOMARY, US_CUSTOMARY_SYSTEM),
     ],
 )
 def test_get_unit_system(key: str, expected_system: UnitSystem) -> None:
@@ -349,7 +360,9 @@ def test_get_unit_system(key: str, expected_system: UnitSystem) -> None:
     assert get_unit_system(key) is expected_system
 
 
-@pytest.mark.parametrize("key", [None, "", "invalid_custom"])
+@pytest.mark.parametrize(
+    "key", [None, "", "invalid_custom", _CONF_UNIT_SYSTEM_IMPERIAL]
+)
 def test_get_unit_system_invalid(key: str) -> None:
     """Test get_unit_system with an invalid key."""
     with pytest.raises(ValueError, match=f"`{key}` is not a valid unit system key"):