Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decorators are ugly #952

Closed
refi64 opened this issue Oct 4, 2015 · 17 comments · Fixed by #2270
Closed

Decorators are ugly #952

refi64 opened this issue Oct 4, 2015 · 17 comments · Fixed by #2270
Assignees

Comments

@refi64
Copy link
Contributor

refi64 commented Oct 4, 2015

(with-decorator inc-decorator (defn addition [a b] (+ a b)))

is ugly. My idea is to add something like:

(defn [inc-decorator] addition [a b] (+ a b)

Thoughts?

@zackmdavis
Copy link
Contributor

As of 016557d, there's a #@ reader macro that may alleviate this concern—

=> (defn increment [f] (fn [&rest args &kwargs kwargs] (inc (apply f args kwargs))))
=> #@(increment (defn addition [a b] (+ a b)))
=> (addition 1 1)
3

@refi64
Copy link
Contributor Author

refi64 commented Oct 4, 2015

I still think the reader macro looks a tad uglier...

@gilch
Copy link
Member

gilch commented Oct 5, 2015

Remember decorators can also apply to classes. I think I like the reader macro better.

@algernon
Copy link
Member

algernon commented Mar 2, 2016

Lots of things are valid lisp, yet, ugly.

I'd have a counter-proposal:

(defn addition ^[@increment] [a b] (+ a b))
(defn addition ^{:decorator increment} [a b] (+a b))

I'd put the decorators after the function name, and use a ^ mark, to make it clear it is not the argument list. I'd also allow the second form, which paves the way for additional meta-data, clojure-style.

(These are just random ideas, I have no preference whatsoever, as I am fine with #@ so far.)

@tuturto
Copy link
Contributor

tuturto commented Mar 2, 2016

Having decorators declared after the function name makes more sense to me. It makes it easier to spot function name in a glance when reading code and I would argue that the function name is more important than decorators being applied to the function. We have to remember to consider the case where function is being decorated with more than one decorators.

(defn addition ^[@increment] [a b] (+ a b)) allows multiple decorators, because of the list structure, but (defn addition ^{:decorator increment} [a b] (+a b)) doesn't. Would (defn addition ^{:decorator [increment multiplication]} [a b] (+a b)) make sense? I'm not a big fan of having a list there, but can't think of any other way of specifying multiple decorators.

@gilch has a good point about being able to decorate classes. It would be nice to have the same syntax on both function and class definition, so one has to remember less of syntax.

(defclass Foo [Bar] ^{:decorator [Baz]} ...) would work I guess. I would put the decorator list at the end for the same reason as with the function definition.

@refi64
Copy link
Contributor Author

refi64 commented May 13, 2016

Still not a syntax fan, but it'd probably be pointlessly stupid to argue further, since my suggested syntax would require more special-casing. Closing!

@refi64 refi64 closed this as completed May 13, 2016
@gilch gilch reopened this Aug 3, 2017
@gilch
Copy link
Member

gilch commented Aug 3, 2017

I'm starting to reconsider this one now that I've started using the #@ version with git. I'm coming around to @kirbyfan64's first suggestion:

(defn [inc-decorator] addition [a b] (+ a b)

We would also need to do that for classes.

(defclass [some-decorator] Foo ...

Think about how lines will change in version control if you have to wrap the whole function. It will mean re-indenting the whole thing when you just added a decorator on top. Python decorators don't have this problem. You could also spread the list over multiple lines to have a good way of changing and re-ordering decorators with version control.

(defn [
       foo
       bar]
  some-function [args]
  ...

@gilch
Copy link
Member

gilch commented Aug 25, 2017

Another reason to change it is for code folding. If you fold everything in your editor, you'd really want the first line of the top level form to have the class or function name. Look what the current decorator syntax does to an excerpt of #1328 when folded:

#@((_dest-setup 'destructure-dict)...)

#@((_dest-setup 'destructure-dict)...)

#@((_dest-setup 'destructure-dict)...)

We can't tell them apart anymore. With the proposed syntax, the first one folds like

(defn [_dest-setup] dest-dict [ddict result extend found binds gsyms]...)

You could even add multiple decorators on the same line, like

(defn [_dest-setup foo bar] dest-dict [ddict result extend found binds gsyms]...)

It's an improvement, but once you take multiple lines, the name disappears when folded.

;; top level fold
(defn [_dest-setup...)

;; unfolded one level
(defn [_dest-setup
       foo
       bar] dest-dict [ddict result extend found binds gsyms]...)

So perhaps the function name should still come first. But then where do the decorators go?

If you put it right after the name, it looks like the arguments list. And a function needs to be able to return a list, so it probably shouldn't go next (even though we do that with docstrings, because Python). So how about putting it first and using @ to distinguish it from the arguments list like this?

(defn dest-dict @[_dest-setup foo bar] [ddict result extend found binds gsyms]...)

When folded and the decorators list is spread over multiple lines, you'd still lose sight of the arguments list--

(defn dest-dict @[_dest-setup...)

--but decorators can change that anyway, so they're more important to put first. Of course, this is optional. When there's no @ after the name, the compiler doesn't add any decorators, and the next list is the args list.

Classes would have to work the same way.

(defclass foo-class @[foo-decorator bar-decorator] [foo-superclass]...)

@Kodiologist
Copy link
Member

You can also just write (setv addition (inc-decorator (fn [a b] (+ a b)))).

@vodik
Copy link
Contributor

vodik commented Apr 2, 2018

@Kodiologist the hole left is how to do class decoration though.

@Kodiologist
Copy link
Member

Hy has no equivalent to fn for classes, but you can say (setv MyClass (mydecorator (type "MyClass" (,) {...}))).

@vodik
Copy link
Contributor

vodik commented Apr 2, 2018

Yeah, I realize, but do you really want that to be the official suggestion?

Though you do have a point... it wouldn't be too hard to provide a class form in the standard library to paper over this and turn the defclass form into an equivalent type or appropriate metaclass call.

Not sure how PEP 487 would fit in, but its an idea.

@gilch
Copy link
Member

gilch commented Apr 2, 2018

Or you can define the class as normal and then afterwards (setv MyClass (mydecorator MyClass)), which is basically what the decorator syntax is sugar for.

An anonymous class doesn't really make sense, since you often need to refer to its name in methods and for inheritance. Even type requires a name argument. It's awkward enough that Hy sometimes gives fn output a .__name__ like '_hy_anon_var_1'. Hy is smart enough to handle (setv foo (fn[] "doc" None)), but not e.g.
(setv (, foo) [(fn[] "doc" None)]). It doesn't come up because it's not used much, but I think this kind of mismatched metadata would be much worse for classes.

I wonder if the class form could return itself instead of None. Then you could just (setv MyClass (mydecorator (class MyClass [] ...))). Actually, that doesn't seem that much easier.

When interfacing with Python libraries, decorators and metaclasses could come up a lot. It probably shouldn't have any more ceremony than Python does. I don't really have a better idea than adding them to the function and class forms. I'm reluctant to make the special forms any more complex than they already are because that complicates let. But these could certainly be implemented as macros that expand to the with-decorator form. We probably don't even need decorators for fn (just nest it). defn and defclass will do.

@vodik
Copy link
Contributor

vodik commented Apr 3, 2018

I wonder if the class form could return itself instead of None. Then you could just (setv MyClass (mydecorator (class MyClass [] ...))). Actually, that doesn't seem that much easier.

I was musing out loud about the later (not really anonymous but more like expressions), and yeah, its not great.

It probably shouldn't have any more ceremony than Python does. I don't really have a better idea than adding them to the function and class forms.

We can revisit this again when #1482 lands. I did start on it, just got stuck needing to fix the importing system (specifically just need self imports to work).

(defn addition
   {:decorate inc-decorator}
   [a b]
   (+ a b))

Isn't too bad...

@gilch
Copy link
Member

gilch commented Apr 3, 2018

Thinking aloud again.
Another syntax idea: just list the decorators after the name and before the []. E.g.

=> (defn foo bar (baz x y) [a b c] ...)
@baz(x, y)
@bar
def foo(a, b, c):
    ...

Decorators are either symbols or HyExpressions. The list is terminated with the arguments HyList. A macro can tell the difference.

This re-orders the same way as ->, which is opposite the order Python writes it. Might be confusing.
If you do it the other way, you lose the obvious function name, especially when code folding:

(defn (baz x y) bar foo [a b c] ...)

Maybe you just indent it differently as a matter of standard style:

(defn (baz x y) 
      bar
      foo [a b c]
  ...)

That looks more like Python, but doesn't help with the folding.

Maybe you could just wrap the name to make it look like a call.

(defn ((baz x y) (bar foo)) [a b c] ...)

That's pretty explicit (reminds me of scheme functions) I feel like rewriting it to use -> because I don't think it's that easy to read.
(defn (-> foo bar (baz x y)) [a b c] ...)
But this isn't quite what -> means. It would expand to (baz (bar foo) x y), which is wrong.

Maybe we could use a @: macro (can't just use @, that's an operator) instead of the #@ tag macro, which could dump its body into a defn or defclass, depending on which keyword is specified:

(@: (baz x y)
    bar
    :defn foo [a b c]
  ...)

Note that the body indent need not be changed now, even if the decorators are removed. This makes it play more nicely with version control. It also looks more like the Python.

(@: :defn foo [a b c]
  ...)
;; same as
(defn foo [a b c]
  ...)

It's also a bit easier to add decorators this way in the editor, since you don't have to wrap more parentheses. Just insert @: : before the defn or defclass then add the decorators in between the :'s.

@gilch
Copy link
Member

gilch commented Apr 3, 2018

{:decorate inc-decorator}

Metadata could work, I suppose. Macros can see it after all. But what if you want to attach more than one? The above syntax doesn't quite work. You'd have to wrap it in a list or something.
{:decorate [(baz x y) bar]}
It would be shorter if you just called it :@.
{:@ [(baz x y) bar]}
That's still a lot of brackets.

@gilch
Copy link
Member

gilch commented May 9, 2021

I ended up making decorator syntax part of the def and class macros in Hebigo.

Although Hebigo's macros are optimized for Hebigo's constraints, which don't always look optimal in S-expressions, I'm pretty happy with how this one looks, even when used in Lissp.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants