-
-
Notifications
You must be signed in to change notification settings - Fork 32k
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.
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
Conversation
fc1bb63
to
4079598
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
@@ -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] |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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):
More context around EventType
(instead of Event
being a generic):
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).
There was a problem hiding this comment.
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.
Proposed change
Improves the typing of floor registry events.
needs #110843
Type of change
Additional information
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.To help with the load of incoming pull requests: