Make embedded statuses directly accessible attributes#1542
Make embedded statuses directly accessible attributes#1542starkillerOG wants to merge 6 commits intorytilahti:masterfrom
Conversation
|
|
|
@rytilahti could you have a look at this PR, it is very small and simple. |
|
Yeah, I was thinking about the naming and I knew it would not be a valid identifier for direct accesses. So, I'm wondering if we should just use double-underscore instead of |
| other_name = str(other.__class__.__name__) | ||
|
|
||
| self._embedded[other_name] = other | ||
| setattr(self, other_name, other) |
There was a problem hiding this comment.
We can add this (and update the docstring to inform anyone reading the docs about the fact), and also change the separator to __ instead of :, in this PR?
There was a problem hiding this comment.
No __ can not be used as seperator, I tried it but very bad things start happening.
Python uses __ for python variables like __main__, __name__ and even worse __getattribute__ itself is also an attribute to the class, so you get into an infinite loop of __getattribute__ getting itself using another __getattribute__ and you crash the program.
There was a problem hiding this comment.
I don't think having a double undescore should be a problem as a separator in the middle of the name, as far as I know the dunder methods need to both start and end with __.
What sort of bad things were you experiencing?
Codecov Report
@@ Coverage Diff @@
## master #1542 +/- ##
==========================================
+ Coverage 80.82% 81.08% +0.25%
==========================================
Files 151 153 +2
Lines 14798 15017 +219
Branches 1696 1713 +17
==========================================
+ Hits 11960 12176 +216
- Misses 2591 2594 +3
Partials 247 247
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
@rytilahti ready for another review/merge |
rytilahti
changed the title
|
Closing this as it has been resolved by #1573 |
3 participants
Consider this test script with a Roborock vacuum:
print(status["CleaningSummary:count"])Will give
TypeError: 'VacuumStatus' object is not subscriptableprint(status.CleaningSummary:count)will give
SyntaxError: invalid syntaxThis is a invalid syntax since variable names can only contain (A-Z, a-z), (0-9), and (_), see https://realpython.com/python-variables/#variable-names
Therefore overwriding getattribute will not work as intended.Alright so it does work when using getattr(status, "CleaningSummary:count") which is used inside HomeAssistant for conveniance. So lets keep it.
This PR will make the embeded classes a attribute of the main class, so you can acess the embedded classes using:
print(status.CleaningSummary.count)Which will work
(This is still needed since using normal python scripts there needs to be an easy way to acces the embeded containers)