fix: Update containers that __post_init__ to use properties#503
fix: Update containers that __post_init__ to use properties#503allenporter merged 5 commits intoPython-roborock:mainfrom
Conversation
This is done since with the new v1 api we reuse attributes and mutate them on update, so tis allows the properties to reamin fresh.
allenporter
force-pushed
the
post_init
branch
from
f822f48 to
cc36704
Compare
Lash-L
left a comment
Lash-L
left a comment
There was a problem hiding this comment.
Should properties be included in the snapshot? Genuine question, no real opinion either way
Good idea. Updated to rewrite repr to include the attributes as well. |
roborock/containers.py
Outdated
| def __repr__(self) -> str: | ||
| return _attr_repr(self, ["start_time", "end_time"]) |
There was a problem hiding this comment.
Would this be simpler/ avoid maintenance if we just added a iterator in the roborockbase, checking if each element is a property and if so we add it to _attr_repr?
There was a problem hiding this comment.
More computational overhead though
There was a problem hiding this comment.
I tried to combine with the existing logic so it won't be more complex overall. The only thing i can't seem to make better is just sticking this in the base class without additional copies in the subclasses. dir in the parent doesn't find the child properties for some reason, but maybe something we can figure out in a future pr
|
|
||
| def _attr_repr(obj: Any, attrs: list[str]) -> str: | ||
| """Return a string representation of the object including specified attributes.""" | ||
| def _attr_repr(obj: Any) -> str: |
There was a problem hiding this comment.
could this just become the default repr for RoborockBase?
There was a problem hiding this comment.
I tried but it seemed to not see child attributes for some reason. (see comment above)
There was a problem hiding this comment.
(It could be that it is specific to the traits with mixins since those are the ones I am testing, but other way, this did seem to address that)
There was a problem hiding this comment.
Ah I see. Thanks, I see your other comment now. Yep we can revisit in the future if needed.
|
Already started doing a version bump on core so i can fix the url issue. But if you want this included, lmk and i can add it. Or we can do a follow up |
This was a change in Python-roborock#503 that changed behavior to omit `0` states, however this is reverting it to preserve compatibility with old behavior. In practice, HomeAssistant uses `none` for matching error codes for example.
This was a change in Python-roborock#503 that changed behavior to omit `0` states, however this is reverting it to preserve compatibility with old behavior. In practice, HomeAssistant uses `none` for matching error codes for example.
2 participants
Update all computed properties to use properties rather than setting during
__post_init__This is done since with the new v1 api we reuse attributes and mutate them on update, so this allows the properties to not get stale with values from a previous post init.