-
-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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 typing of deCONZ diagnostics #69491
Improve typing of deCONZ diagnostics #69491
Conversation
homeassistant/components/deconz/diagnostics.py:28: error: Item "None" of "Optional[WSClient]" has no attribute "state" [union-attr] homeassistant/components/deconz/diagnostics.py:40: error: Unpacking a string is disallowed [misc] homeassistant/components/deconz/diagnostics.py:40: error: Cannot determine type of "k" [has-type] homeassistant/components/deconz/diagnostics.py:40: error: Cannot determine type of "v" [has-type] homeassistant/components/deconz/diagnostics.py:42: error: Unpacking a string is disallowed [misc] homeassistant/components/deconz/diagnostics.py:42: error: Cannot determine type of "k" [has-type] homeassistant/components/deconz/diagnostics.py:42: error: Cannot determine type of "v" [has-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.
The code changes look good to me.
The title of the PR doesn't seem to fit the change?
I assume that there are either no collisions merging these dicts or that the collisions don't matter. Is that true?
Isn't the PR improving the typing of deconz diagnostics platform? :) Exactly. The end result should look the same. There are no duplication of data between those different classes as the data will only be put into one of them |
I suppose I was looking for type hints etc. But yes I suppose so! |
Thanks @davet2001 |
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.
This is a case where maintainability is hindered by Mypy, in that you now have to remember to update this function if the api.lights
and api.sensors
groups ever change. It might be better to stick with the original code that uses groups, and cast as needed.
… library which I will do once I resolve as much as possible in the integration
No worries! Thanks for working in the deconz integration, it's the first thing I ever connected HA to.
|
Thanks for using it and giving back to the community! |
Breaking change
Proposed change
Full typing was done here #58968 and part of it split out to this PR.
Type of change
Additional information
Checklist
black --fast 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
.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: