Hi there, I’m diving into the AppDaemon code to better understand it, and maybe help out implementing my own feature request.
I see many parts of the code that aren’t really “pythonic”, and after many years of python programming it kinda hurts my eyes seeing this practices 
As an example:
for name in self.app_config:
if "disable" in self.app_config[name] and self.app_config[name]["disable"] is True:
...
- looping dictionary keys to then get the value by that key is best done with
dictionary.items()that already returns the key and value pair - the dictionary key existence can be bypassed by using
dictionary.get(key, default)(or even without the default value since it returns None by …default) - the
if condition is Truecan be simplified inif conditionto leverage the boolness of any object; it will also evaluate to true for empty lists, dictionary and strings.
With that in mind, the code can be rewritten to
for name, cfg in self.app_config.items():
if cfg.get("disable"):
...
Also, many methods are really long and it’s difficult to track what’s going on.
This post is not to criticize what’s been done so far, but it is simply to ask:
- where this coding style decisions made because of better performances? In my projects I prefer readability over optimization, so I don’t know if there’s some benchmarking done behind this
- If the answer of the previous question is no, are you open to accept PRs that alter significantly the code to make it “cleaner”?
I personally leverage the sourcery plugin (for vscode, pycharm and other IDEs) to detect “bad” code and refactor it, it is really handy for this kind of work.
Other than the style, I also have an architecture related question: why use the AppDaemon class to hold all the other classes and pass it to them, instead of explicitly define and store the dependencies of a component inside its __init__ method?
If i understand correctly, the components are created only once, so the self.AD.component will always return the same instance.
A classmethod from_appdaemon could still accept the AppDaemon instance, but just to get the dependencies and pass them to the init method.
With this small, but big change, unit testing would be much easier (one can mock the dependencies and focus on the testing of the component functionality)