Allow to stack decorators

[jourdain]

Thanks for building such awesome library but I wanted to see if I can use it within trame in a more streamline fashion.

Right now it is working if I create an indirection like in that working example but ideally I would like to be able to stack decorators like in the snippet below.

@ctrl.set("update_message")
@reloading
def update_message():
    # ...

But when I do, I get the following exception

  File "reloading.py", line 247, in wrapped
    state["func"] = get_reloaded_function(caller_globals, caller_locals, fpath, fn) or state["func"]
  File "reloading.py", line 222, in get_reloaded_function
    code = get_function_def_code(fpath, fn)
  File "reloading.py", line 214, in get_function_def_code
    found = isolate_function_def(fn.__name__, tree)
  File "reloading.py", line 201, in isolate_function_def
    and "reloading" in [
  File "reloading.py", line 202, in <listcomp>
    get_decorator_name(dec)
  File "reloading.py", line 184, in get_decorator_name
    return dec_node.func.id
AttributeError: 'Attribute' object has no attribute 'id'

It is possible that the issue is on my side (@ctrl, @state) but any help will be greatly appreciated.

[psavery]

To follow up, there were only a few things that I needed to modify to get this to work for me and @jourdain. Here's a diff:

diff --git a/reloading/reloading.py b/reloading/reloading.py
index aef1353..8401a81 100644
--- a/reloading/reloading.py
+++ b/reloading/reloading.py
@@ -181,14 +181,17 @@ def _reloading_loop(seq, every=1):
 def get_decorator_name(dec_node):
     if hasattr(dec_node, "id"):
         return dec_node.id
-    return dec_node.func.id
+    elif hasattr(dec_node.func, "id"):
+        return dec_node.func.id
+    return dec_node.func.value.id

 def strip_reloading_decorator(func):
-    """Remove the reloading decorator in-place"""
-    func.decorator_list = [
-        dec for dec in func.decorator_list if get_decorator_name(dec) != "reloading"
-    ]
+    """Remove the 'reloading' decorator and all decorators before it"""
+    decorator_names = [get_decorator_name(dec) for dec in func.decorator_list]
+    reloading_idx = decorator_names.index("reloading")
+    func.decorator_list = func.decorator_list[reloading_idx + 1:]

Basically, the method to get the decorator name for @ctrl.set("update_message") was not working for some reason. But adding a check for "id" on the func object, and then trying dec_node.func.value.id, worked for us (this returns ctrl).

The second part of our issue is that on the current master branch, all decorators are reloaded except the @reloading decorator. I believe, however, it may make more sense to only reload decorators after the @reloading decorator. This is because if we set up decorators like this:

@decorator
@reloading
def func():
    pass

Then the @decorator there is going to be taking the reloading-wrapped function at first, but the first time it is called, this reloading-wrapper gets removed since it is reloaded without the @reloading.

This was the issue with our code. The @ctrl.set("update_message") decorator is setting the function to a global object, and when it is first defined, the function is the reloading-wrapped function. But as soon as it gets called for the first time, the @ctrl.set("update_message") is executed again, and it replaces the reloading-wrapped function with a static, non-reloading function. So the reloading would no longer work after the first time.

I also think it logically makes more sense to only reload what comes after @reloading.

@julvo If you agree with these changes, I am happy to put up a PR.

[julvo]

Thank you both for reporting and looking into this.

Both suggested changes make sense to me and I'd be happy to merge a PR with these.

For the decorator name lookup in the first change: I guess we could be even more defensive here, name the function get_decorator_name_or_none and check that any attribute exists before we access it and return None otherwise instead of crashing.

[psavery]

@julvo Great, thank you! I just made a PR that includes your suggestion.