[Feature Proposal]: Code to support `**kwargs` in callbacks

[mementum]

As many others I was puzzled at first when reading the documentation and seeing that kwargs was being used without ** in the callbacks. And soon I found out that, indeed, it was not the standard keyword arguments what was being used there, but an argument named kwargs, which is a dict containing things the user want to have passed to the callback and some arguments which convey internal information about appdaemon.

Here and there I read the background as to why it started so and that sometimes it was considered how to move to the standard **kwargs

Being fond myself, used to python, of **kwargs I devised this subclass of Hass which helps me use **kwargs in my code (see below)

It features 2 settings to keep compatibility:

• USE_KWARGS which determines if the subclass wants to use standard python **kwargs
• DEL_INTERNALS which determines if things like __thread_id are removed from kwargs

My code uses a subclass thereof in which both settings are set to True. But the default settings are set to False for compatibility with actual appdaemon behavior.

I wonder whether work in this direction could be integrated into the code or I shall keep this to myself. (So far only the methods which have played a role in my code have been subclassed)

import hassapi as hass

class Hass(hass.Hass):

    USE_KWARGS = False
    DEL_INTERNALS = False

    # cb faker, to bypass the named 'kwargs' argument which isn't a real
    # **kwargs.
    # it's presence its guaranteed as the last named argument, so one can take
    # the named arguments [0:-1] and then [-1] (-1 will always exist), knowing
    # it's a dictionary and convert it to **kwargs
    def _cb(self, realcb):
        if not self.USE_KWARGS:
            return realcb  # use standard AppDaemon kwargs convention

        def _cb(*args):
            args, kwargs = list(args[:-1]), args[-1]

            if self.DEL_INTERNALS:  # user doesn't want cb with extra info
                kwargs = {k: v for k, v in kwargs.items()
                          if not(isinstance(k, str) and k.startswith('__'))}

            if len(args) == 4:  # state callback (4 after conversion above)
                # set conflicting args in the expected positions and remove
                # the conflict
                for i, x in enumerate(('attribute', 'old', 'new'), 1):
                    args[i] = kwargs.pop(x, args[i])

            return realcb(*args, **kwargs)

        # Needed to identify the callback in the admin user-interface/logs
        _cb.__name__ = realcb.__name__

        return _cb

    # Methods overridden so far to support the callback faker
    def listen_state(self, cb, *args, **kwargs):
        return super().listen_state(self._cb(cb), *args, **kwargs)

    def run_at(self, cb, *args, **kwargs):
        return super().run_at(self._cb(cb), *args, **kwargs)

    def run_daily(self, cb, *args, **kwargs):
        return super().run_daily(self._cb(cb), *args, **kwargs)

    def run_in(self, cb, *args, **kwargs):
        return super().run_in(self._cb(cb), *args, **kwargs)

    def run_once(self, cb, *args, **kwargs):
        return super().run_once(self._cb(cb), *args, **kwargs)

    def register_service(self, svc, cb, *args, **kwargs):
        return super().register_service(svc, self._cb(cb), *args, **kwargs)

[Odianosen25]

Hello @mementum,

Yeah also got your point, and as you noted there been some discussions abt moving to using **kwargs; but worries over how not to negatively affect lots of users (virtually all users) has been a worry.

Don’t know if you aware, but callbacks could also take cb(*args, **kwargs). Not in the documentation I believe and not sure if we I intentionally left it out 🤔.

But anyways it’s an option if you want to explore.

[ReneTode]

odia, i just realised because of this, that maybe in the docs everywhere where kwargs is used what is just a dict, it could be replaced with something like "CB-vars"

that would at least get rid of the confusion for "python users" and it would be easy because its just a doc change.

[mementum]

Don’t know if you aware, but callbacks could also take cb(*args, **kwargs). Not in the documentation I believe and not sure if we I intentionally left it out 🤔.

As far as I understand it, this is because in previous versions of appdaemon the actual signature of the callback was being check ex-ante. But using *args, **kwargs won't actually provide the user supplied named arguments as **kwargs. The last argument in *args will still be a dictionary containing those and the internals of appdaemon like __thread_id

odia, i just realised because of this, that maybe in the docs everywhere where kwargs is used what is just a dict, it could be replaced with something like "CB-vars"

That would at least clear some of the existing confusion. See for example here:

• https://appdaemon.readthedocs.io/en/latest/APPGUIDE.html#state-callbacks

The callback is defined as:

def my_callback(self, entity, attribute, old, new, kwargs):

but scrolling down when it comes to say what kwargs is the following is what the user can read

**kwargs A dictionary containing any constraints and/or additional user specific keyword arguments supplied to the listen_state() call.

The declaration does not use the double asterisk, but the definition does, which is (at least for me) confusing.

[ReneTode]

the double asterix in that part from the docs is indeed wrong, and should be corrected. and i think renaming it in the docs would take away all confusion for python users that are used to kwargs.

[acockburn]

OK, I think I have reached a solution for this. AS of 4.3.0, it will be possible to globally switch AD to use of **kwargs in callbacks, with the default still being the old mechanism. Docs have been updated to show the new mechanism, and all instances of kwargs as a dict have been renamed to cb_vars

[mementum]

The kwargs branch does still have (at least) an error. See https://github.com/AppDaemon/appdaemon/commit/c381b0f5b6f123b83387fa9bd6a4916963ccb8fc

[acockburn]

Thanks - I’ll fix it shortly.