Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve floor registry event typing #110844

Merged
merged 1 commit into from
Feb 18, 2024
Merged

Improve floor registry event typing #110844

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions homeassistant/helpers/area_registry.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import dataclasses
from typing import Any, Literal, TypedDict, cast

from homeassistant.core import Event, HomeAssistant, callback
from homeassistant.core import HomeAssistant, callback
from homeassistant.util import slugify

from . import device_registry as dr, entity_registry as er
Expand Down Expand Up @@ -344,12 +344,14 @@ def _async_setup_cleanup(self) -> None:
from . import floor_registry as fr # Circular dependency

@callback
def _floor_removed_from_registry_filter(event: Event) -> bool:
def _floor_removed_from_registry_filter(
event: fr.EventFloorRegistryUpdated,
) -> bool:
"""Filter all except for the remove action from floor registry events."""
return bool(event.data["action"] == "remove")
return event.data["action"] == "remove"

@callback
def _handle_floor_registry_update(event: Event) -> None:
def _handle_floor_registry_update(event: fr.EventFloorRegistryUpdated) -> None:
"""Update areas that are associated with a floor that has been removed."""
floor_id = event.data["floor_id"]
for area_id, area in self.areas.items():
Expand All @@ -358,8 +360,8 @@ def _handle_floor_registry_update(event: Event) -> None:

self.hass.bus.async_listen(
event_type=fr.EVENT_FLOOR_REGISTRY_UPDATED,
event_filter=_floor_removed_from_registry_filter,
listener=_handle_floor_registry_update,
event_filter=_floor_removed_from_registry_filter, # type: ignore[arg-type]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need the type ignores, I don't remember?

Copy link
Member Author

@frenck frenck Feb 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Little bit of context to what made me do this (and the adjustment of typing of fire event data):

#110821 (review)

More context around EventType (instead of Event being a generic):

#97071

Specifically this discussion: https://github.com/home-assistant/core/pull/97071/files#r1271450383

I personally don't prefer the ignores either, but... if support for defaults in typevar is landing/added, all these ignores will go away (and be caught by the linters as obsolete).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've already added the basic support for TypeVar defaults to Mypy. That's all that we would need here. Just waiting for a new release now before I can start to update the typing. Should have been out a week ago, so hopefully next week 🤞🏻

I suggest to use type: ignore until then. Will be simple to remove.

python/mypy#16843

listener=_handle_floor_registry_update, # type: ignore[arg-type]
)


Expand Down
31 changes: 26 additions & 5 deletions homeassistant/helpers/floor_registry.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
from collections.abc import Iterable, ValuesView
import dataclasses
from dataclasses import dataclass
from typing import cast
from typing import Literal, TypedDict, cast

from homeassistant.core import HomeAssistant, callback
from homeassistant.util import slugify

from .typing import UNDEFINED, UndefinedType
from .typing import UNDEFINED, EventType, UndefinedType

DATA_REGISTRY = "floor_registry"
EVENT_FLOOR_REGISTRY_UPDATED = "floor_registry_updated"
Expand All @@ -19,6 +19,16 @@
SAVE_DELAY = 10


class EventFloorRegistryUpdatedData(TypedDict):
"""Event data for when the floor registry is updated."""

action: Literal["create", "remove", "update"]
floor_id: str


EventFloorRegistryUpdated = EventType[EventFloorRegistryUpdatedData]


@dataclass(slots=True, kw_only=True, frozen=True)
class FloorEntry:
"""Floor registry entry."""
Expand Down Expand Up @@ -151,7 +161,10 @@ def async_create(
self.async_schedule_save()
self.hass.bus.async_fire(
EVENT_FLOOR_REGISTRY_UPDATED,
{"action": "create", "floor_id": floor_id},
EventFloorRegistryUpdatedData(
action="create",
floor_id=floor_id,
),
)
return floor

Expand All @@ -160,7 +173,11 @@ def async_delete(self, floor_id: str) -> None:
"""Delete floor."""
del self.floors[floor_id]
self.hass.bus.async_fire(
EVENT_FLOOR_REGISTRY_UPDATED, {"action": "remove", "floor_id": floor_id}
EVENT_FLOOR_REGISTRY_UPDATED,
EventFloorRegistryUpdatedData(
action="remove",
floor_id=floor_id,
),
)
self.async_schedule_save()

Expand Down Expand Up @@ -196,7 +213,11 @@ def async_update(

self.async_schedule_save()
self.hass.bus.async_fire(
EVENT_FLOOR_REGISTRY_UPDATED, {"action": "update", "floor_id": floor_id}
EVENT_FLOOR_REGISTRY_UPDATED,
EventFloorRegistryUpdatedData(
action="update",
floor_id=floor_id,
),
)

return new
Expand Down
Loading