Decorators support for defasync and anonymous functions

[ikappaki]

Hi,

Currently, there doesn't seem to be a way to decorate a function defined with defasync.

The defn macro supports a :decorations attr-map? key, but while defasync is built on top of defn, it only considers the :decorations key in the attr-map? and not in metadata.

As a result, passing a metadata :decorator key, such as in (defasync ^{:decorators [decorator]} something [] ...), has no effect.

Would it be advisable to also support a ^{:decorators [...]} meta key for defasync?

Furthermore, when adapting examples from Python to Basilisp, developers often encounter decorated functions without necessarily understanding how they are internally defined.

For anonymous functions, decorating them is not immediately intuitive. A developer with basic knowledge of decorators might deduce that the anonymous function can be passed as the first argument to the decorator, e.g., (decorator (fn [] ...)). However, if the decorator accepts arguments, it must first be instantiated with those arguments before the anonymous function is passed, e.g., ((decorator arg1 arg2) (fn [] ...)).

Given that this requirement assumes Basilisp developers would need to understand some internal details about decorator (something that is not necessarily expected when using Basilisp, especially when coming from a Clojure background) would it be better to extend support for the :decorators meta key to anonymous functions as well? This enhancement could be particularly valuable when transpiling Python code to Basilisp, making it more intuitive and easier to visually pair code between the two languages. Alternatively, we can just extend the documentation to indicate how to manually decorate anonymous fns.

With that in mind, my suggestions would look something like this:

(defasync ^{:decorators [decorator1 (decorator2 arg1 ..) ...] fn-nane [] ...)

(fn ^{:decorators [decorator1 (decorator2 arg1 ...) ...]  _fn-name [] ...)

I have conducted a brief survey below to show the current status. I've decided to create the decorator in python to ensure I don't overlook anything:
asyncdec.py

import asyncio
import random
import time

def atimer(message):
    def decorator(func):
        async def wrapper(*args, **kwargs):
            start_time = time.perf_counter()
            result = await func(*args, **kwargs)
            elapsed_time = time.perf_counter() - start_time
            print(f"[{message}] function '{func.__name__}' executed in {elapsed_time:.2f} seconds")
            return result
        return wrapper
    return decorator

@atimer("hi")
async def acount():
    await asyncio.sleep(random.random())  # Simulate a delay
    return [1, 2, 3]

async def main():
    result = await acount()
    print(result)

if __name__ == "__main__":
    asyncio.run(main())    

run:

> python .\asyncdec.py
[hi] function 'acount' executed in 0.56 seconds
[1, 2, 3]

And then I imported the python atimer decorator for the survey
issue.lpy

(import asyncio
        asyncdec)

;; `defasync`
;;
;; No way to decorate it with an async decorator?
(defasync
  count1 []
  (await (asyncio/sleep (rand)))
  [1 2 3])

(defasync main1
  []
  (println :result (await (count1))))

(asyncio/run (main1))

;; async `defn`
;;
;; Use of the `:decorators` attr-map? option
(defn count2
  {:decorators [(asyncdec/atimer "hey2")]
   :async true}
  []
  (await (asyncio/sleep (rand)))
  [1 2 3])

(defasync main2
  []
  (println :result (await (count2))))

(asyncio/run (main2))

;; Async anonymous function  
;;  
;; An anonymous function can be passed as the first parameter to an instantiated[1] decorator.  
;;  
;; [1] If a decorator accepts arguments, it needs to be instantiated first with those arguments  
;; before passing the anonymous function. In the example below, the decorator accepts a single argument,  
;; so it is instantiated first, then the anonymous function is passed to it.
(def count3
  ((asyncdec/atimer "hey3")
   (fn ^{:async true} _count3 []
     (await (asyncio/sleep (rand)))
     [1 2 3])))

(defasync main3
  []
  (println :result (await (count3))))

(asyncio/run (main3))

run:

> basilisp run issue.lpy
:result [1 2 3]
[hey2] function '__count2_113' executed in 0.11 seconds
:result [1 2 3]
[hey3] function '___count3_119' executed in 0.29 seconds
:result [1 2 3]

[chrisrink10]

The defn macro supports a :decorations attr-map? key, but while defasync is built on top of defn, it only considers the :decorations key in the attr-map? and not in metadata.

As a result, passing a metadata :decorator key, such as in (defasync ^{:decorators [decorator]} something [] ...), has no effect.

Would it be advisable to also support a ^{:decorators [...]} meta key for defasync?

By my reading, defn also does not support ^{:decorators [...]} on the name metadata. Fixing that behavior would automatically fix defasync, yes?

Furthermore, when adapting examples from Python to Basilisp, developers often encounter decorated functions without necessarily understanding how they are internally defined.

For anonymous functions, decorating them is not immediately intuitive. A developer with basic knowledge of decorators might deduce that the anonymous function can be passed as the first argument to the decorator, e.g., (decorator (fn [] ...)). However, if the decorator accepts arguments, it must first be instantiated with those arguments before the anonymous function is passed, e.g., ((decorator arg1 arg2) (fn [] ...)).

Given that this requirement assumes Basilisp developers would need to understand some internal details about decorator (something that is not necessarily expected when using Basilisp, especially when coming from a Clojure background) would it be better to extend support for the :decorators meta key to anonymous functions as well? This enhancement could be particularly valuable when transpiling Python code to Basilisp, making it more intuitive and easier to visually pair code between the two languages. Alternatively, we can just extend the documentation to indicate how to manually decorate anonymous fns.

I am skeptical about this argument personally, especially since it's being made for asynchronous decorators which are fraught even for experienced developers.

That being said, I would accept a patch to move the support for :decorators down into fn, which would then cover all these cases (defn, defasync, etc.).

[ikappaki]

By my reading, defn also does not support ^{:decorators [...]} on the name metadata. Fixing that behavior would automatically fix defasync, yes?

I believe so, yes.

I am skeptical about this argument personally, especially since it's being made for asynchronous decorators which are fraught even for experienced developers.

That being said, I would accept a patch to move the support for :decorators down into fn, which would then cover all these cases (defn, defasync, etc.).

Sure, I will look into it, thanks

[ikappaki]

That being said, I would accept a patch to move the support for :decorators down into fn, which would then cover all these cases (defn, defasync, etc.).

Sure, I will look into it, thanks

Hi @chrisrink10 , I looked into the fn macro in basilisp.core, and it doesn't appear to share a common base with defn, which is built on top of def. Therefore, I don't think it's possible to move the support for :decorators down into fn as I understood you meant it?

(def ^:macro ^:redef fn
  (fn* fn [&env &form & decl]
       (with-meta
         (cons 'fn* decl)
         (meta &form))))

;; ...

;; inside the `defn` macro
... `(def ~fname ~fn-body))))

However, they seem to share a common path though in generator.py where _def_to_py_ast() calls _fn_to_py_ast(). The latter looks like a potential candidate for handling decorators in a shared way.

I'd also like to add that after the fix for #1187, the :decorators meta key is supported in defasync, and any macro defined over defn.

Based on the latest findings, I see the following options moving forward

1. defn and all macros defined over it can use the :decorators meta key to apply decorators. For fn, the caller can use the (decorator (fn [] ...) idiom. We can add a new section in the documentation about Python decorators support and include tests exercising the decorators paths (currently none).
2. Same as above, but also support the :decorators meta key in fn, independently of defn.
3. Same as above, but extract the decorators handling code from defn into a shared macro/function that fn can also use.
4. Move decorators support down to the generator's _fn_to_py_ast() function.

I think ideally the order of preference should be reversed from the list above: option 4 will be the most ideal, followed by option 3 and so on. Practically, though, it seems better to start with option 1 and see how far we can go up the list?

What do you think?

Thanks

[ikappaki]

ah right, I just realised that the decoration handling code in defn creates a fn

fn-body    (if decorators
                       (loop [wrappers (seq (python/reversed decorators))
                              final    `(fn ~fname ~@body)]
                         (if (seq wrappers)
                           (recur (rest wrappers)
                                  `(~(first wrappers) ~final))
                           final))
                       `(fn ~fname ~@body))

So most probably what you meant is to pass in the :decorations meta to the fn and let it deal with it, something like

fn-body    (if decorators
             `(fn ^{:decorators ~decorators} ~fname ~@body)
             `(fn ~fname ~@body))

[chrisrink10]

So most probably what you meant is to pass in the :decorations meta to the fn and let it deal with it [...]

The metadata should already be being passed into fn (via the meta on the name), so it should not require explicitly doing so. I'm suggesting moving all of the handling of (:decorators fmeta) into the redefined fn macro here. This is how destructuring is added transparently to defn.

You'll probably want to inspect both the name and the &form meta however since fn does not require a name.

[chrisrink10]

Fixed in #1189