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.

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

Switch from attrs to dataclasses #36

Merged
merged 11 commits into from
Jun 29, 2021
Merged

Switch from attrs to dataclasses #36

merged 11 commits into from
Jun 29, 2021

Conversation

OttoWinter
Copy link
Member

Python attrs was used in the time when we still had to work with python 3.5

In Python 3.7+, we have python's stdlib dataclasses.

They're mostly the same (but dataclass lacks converter functionality, which is manually added with a metaclass), but using the stdlib feels cleaner.

Note: this is a pretty big API breaking change, so would need major version bump, and some changes in HA.

@OttoWinter OttoWinter marked this pull request as draft June 17, 2021 20:11
@OttoWinter OttoWinter marked this pull request as ready for review June 29, 2021 13:35
@OttoWinter OttoWinter merged commit 872c643 into master Jun 29, 2021
@OttoWinter OttoWinter deleted the use-dataclasses branch June 29, 2021 13:36
@@ -284,7 +256,7 @@ def on_msg(msg: message.Message) -> None:
if isinstance(msg, CameraImageResponse):
data = image_stream.pop(msg.key, bytes()) + msg.data
if msg.done:
on_state(CameraState(key=msg.key, image=data))
Copy link
Member

Choose a reason for hiding this comment

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

The issue people are experiencing on HA 2021.7 is because the new from_pb function only matches the attributes in the dataclass by exact name. That is image in CameraState but in CameraImageResponse it is data. Simple fix is to rename the attr to data and the references inside HA

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants