Feature: Simplify Fn args and support positional only args

[allison-casey]

Closes #1411
This is a quick and dirty implementation of what we were talking about in #1411. I haven't updated any tests, documentation, or core method definitions since I wanna get y'alls opinions on this before jumping into that mammoth task.

To paraphrase the discussion:

• / demarks the end of positional only arguments
• * demarks the start of keyword only arguments
• ^ann #* <sym> collects varargs into <sym> and also marks the start keyword only arguments
• ^ann #** <sym> collects keyword varargs into <sym>
• ^ann <sym> demarks a positional argument
• ^ann [<sym> <default>] demarks a keyword argument
• and there is no more shorthand for a keyword defaulting to None

with function definitions looking like this:

(defn afunc [a ^int b / c [d 1] * ^str e ^int [f 2] g ^str #** kwargs])

with the equivalent python looking like

def afunc(a, b: int, /, c, d=1, *, e: str, f: int=2, g, **kwargs: str): pass

[Kodiologist]

There are a lot of other PRs in the pipeline right now. Let's get those in before investing too much in a change this big.

[allison-casey]

My thought too, just wanted to get this out there early since it would be a big change.

[Kodiologist]

Making this change might be a good opportunity to rename defn to def. That was a case where we pointlessly imitated Clojure when Python already had a perfectly good convention.

[allison-casey]

It does decouple defn from fn name wise which is something to think about (python doesn't relate the names of def and lambda so probably not a huge deal), but I do like converging closer to python

[Kodiologist]

Yeah, whereas I think fn is the better name than lambda because it's a lot shorter, and anonymous functions should be convenient to write in Lisp.

[allison-casey]

I would be happy with changing to def and keeping fn. Would like to hear from @scauligi and @peaceamongworlds about a change like that as well though.

[scauligi]

I don't have a strong opinion either way; speaking strictly for my own personal opinion, I'm more familiar with python than any lisp-family so def is more comfortable to me, but I adapted pretty quickly to using defn when I started writing Hy code so ¯\_(ツ)_/¯

[Kodiologist]

It's still easy to typo def for defn or vice versa when switching rapidly between Hy and Python, as one sometimes needs to do. That's the real value of avoiding needless Python–Hy inconsistencies, in my opinion.

[scauligi]

It's still easy to typo def for defn or vice versa

Can confirm, I have indeed done that several times.

[peaceamongworlds]

I would be happy with changing to def and keeping fn. Would like to hear from @scauligi and @peaceamongworlds about a change like that as well though.

I would prefer to keep defn.

I know this is different from python, but in my mind it just makes more sense. In general, I think that it makes sense for all def* macros to have the name connection that defn and fn have. It also stays more in line with other lisps and doesn't introduce what seems to be just a superficial change.

[peaceamongworlds]

Python itself seems to be inconsistent with how you has to define things: you use = to define variables, def to define functions, and class to define classes. All of those seem to follow a different convention.

Having defn for functions, defclass for classes, defmacro for macros etc seems to be more logical.

[allison-casey]

I think @peaceamongworlds makes a good point. Changing to def kind of negates the reasoning for having defclass instead of just class and there's no real equivalent to defmacro to change to. The semantic unity of fn, defn, defmacro, and defclass is quite nice to have in a language, but so is sticking to the "lipstick on python" philosophy so i'm torn at the moment.

There doesn't seem to be any pushback against the actual changes to the argument processing logic, can I go ahead and start migrating the rest of the repo over to the new fn args?

[Kodiologist]

Actually, I'd advocate changing defclass to class, for the same reason. defmacro, on the other hand, has no Python equivalent, so I'm equally fine with defmacro, macro, or mac a la Arc. I guess defmacro is the most obvious choice, since it seems to be conventional for Lisp.

can I go ahead and start migrating the rest of the repo over to the new fn args?

Yep, no reason to let our bikeshedding of the name stand in the way of that work.

[allison-casey]

This migrates everything. HyDomain hasn't been updated with this or the tag macro changes so the docs will be broken until i can fix that. This shouldn't be merged until then.
I'll squash and reorg the commits when that's done. Some tests in destructure and walk don't really make sense and should probably be changed, but everything still works so that's probably a job for another pr.

[allison-casey]

I have a fix for both tag macros and the new arg format in hydomain sitting on develop. Once we approve this i'll merge that and then we can merge this.

[Kodiologist]

x not in ("/", "*", "#*", "#**")

Isn't #* already an invalid symbol at the parser level?

You don't seem to allow default arguments for positional-only parameters, but Python does.

The pattern a("*") will match e.g. a string, not just a symbol. Similarly, I'd write rest == "*" as rest == HySymbol("*") for the sake of being future-proof.

Are you sure you want maybe(many(OPTIONAL_ANNOTATION + (NASYM | brackets(NASYM, FORM)))) instead of just many(OPTIONAL_ANNOTATION + (NASYM | brackets(NASYM, FORM)))? many already allows 0 arguments. Perhaps you should nest this many in the previous item, because it only makes sense to match arguments here if you already saw a * or #* args first.

Also, Python forbids def f(*): … ("SyntaxError: named arguments must follow bare *"), so we should probably forbid that too or else I think we'll break hy2py.

# Can't have a positional arg after a default arg outside of kwonly list

I think this can be enforced by changing the pattern.

I'll look at the other commits once you've reorganized them.

[allison-casey]

Isn't #* already an invalid symbol at the parser level?

Turns out it is, removed from NASYM list

You don't seem to allow default arguments for positional-only parameters, but Python does.

fixed

The pattern a("") will match e.g. a string, not just a symbol. Similarly, I'd write rest == "" as rest == HySymbol("*") for the sake of being future-proof.

done

Are you sure you want maybe(many(OPTIONAL_ANNOTATION + (NASYM | brackets(NASYM, FORM)))) instead of just many(OPTIONAL_ANNOTATION + (NASYM | brackets(NASYM, FORM)))? many already allows 0 arguments. Perhaps you should nest this many in the previous item, because it only makes sense to match arguments here if you already saw a * or #* args first.

Tried nesting it in the previous form, but it created a lot of funky edge cases with how funcparser lib formed them together. Simplified the form to be more explicit instead.

I think this can be enforced by changing the pattern.

With how posonly and args are parsed separately by funcparserlib, there's no way i've found for the parser to know that it hit a default arg in posonly before it parses args. Plus this way we get more descriptive error messages which is always nice.

[Kodiologist]

Nice, looking better.

Can you break some of the longer lines? Aim for a line length of 70 or 80 characters. 90 or more is too long unless you're shoving something uninteresting onto the end there.

I think posonly is not None and not posonly would be more readable as not (posonly or posonly is None).

Why are you using None as the first argument of _syntax_error instead of expr?

should_be_defaults = dropwhile(lambda x: isinstance(x[1], HySymbol), [*posonly, *args])
if any(isinstance(arg[1], HySymbol) for arg in should_be_defaults):
    raise self._syntax_error(None, "non-default argument follows default argument")

You're writing out the same function twice here. A more concise approach would be:

if any(dropwhile(lambda x: x,
        (isinstance(x[1], HySymbol) for x in posonly + args))):
    raise self._syntax_error(None, "non-default argument follows default argument")

I think you can say just rest and kwargs in place of rest is not None and kwargs is not None.

[scauligi]

Looks like tests failing because of cfor slipping in ahead of this, can you rebase/fix and push?

[allison-casey]

Can you break some of the longer lines? Aim for a line length of 70 or 80 characters. 90 or more is too long unless you're shoving something uninteresting onto the end there.

Done

not (posonly or posonly is None)

Done

Why are you using None as the first argument of _syntax_error instead of expr?

you right. in adding this though I ended up changing

if any(dropwhile(lambda x: x,
        (isinstance(x[1], HySymbol) for x in posonly + args))):
    raise self._syntax_error(None, "non-default argument follows default argument")

to

is_positional_arg = lambda x: isinstance(x[1], HySymbol)
invalid_non_default = next(
    (arg
     for arg in dropwhile(is_positional_arg, posonly + args)
     if is_positional_arg(arg)),
    None
)
if invalid_non_default:
    raise self._syntax_error(
        invalid_non_default[1], "non-default argument follows default argument"
    )

which is obviously more verbose, but we get nicer syntax errors about which arg is the culprit ie:

  File "<stdin>", line 1
    (defn test [a / [b 1] c])
                          ^
hy.errors.HySyntaxError: non-default argument follows default argument

Looks like tests failing because of cfor slipping in ahead of this, can you rebase/fix and push?

Done

[Kodiologist]

Sounds good. So you want to change hydomain like you said earlier and then we'll return to this?

[allison-casey]

hydomain has been updated. tested it on this branch and things look good

[Kodiologist]

Can you reorganize the commits to separate out more substantive changes (like the edits to the body of lambda-list) from the mass syntax changes?

[asemic-horizon]

As it stands now Hy works almost seamlessly with Clojure editor modes. Given the philosophy of “lipstick on Python”, I don’t see why the lipstick should be nude-colored rather than Clojure-colored. Also: while it’s true that one should expect discontinuities for a project like Hy, do the benefits of breaking older code (in the case of `defn`, apparently aesthetic) outweigh the costs? Em 27 de fev. de 2021 13:22 -0300, Allie Jo Casey ***@***.***> escreveu:

…

I think @peaceamongworlds makes a good point. Changing to def kind of negates the reasoning for having defclass instead of just class and there's no real equivalent to defmacro to change to. The semantic unity of fn, defn, defmacro, and defclass is quite nice to have in a language, but so is sticking to the "lipstick on python" philosophy so i'm torn at the moment. There doesn't seem to be any pushback against the actual changes to the argument processing logic, can I go ahead and start migrating the rest of the repo over to the new fn args? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[Kodiologist]

Backwards compatibility isn't in scope until Hy 1.0.

[allison-casey]

I was going to propose a 1.0 feature lock and moving to pre-releases once i'm done categorizing all the open issues to make this drive to 1.0 less painful for users. The constant breaking changes is basically a giant neon sign saying "don't use this language" which probably isn't great for Hy's adoption long term.

[allison-casey]

Can you reorganize the commits to separate out more substantive changes (like the edits to the body of lambda-list) from the mass syntax changes?

Can you be more specific? There's currently one commit for the compiler changes, one for changes to core, and one for the documentation.

[Kodiologist]

The many trivial changes like

- (defmacro seq [param &rest seq-code]
+ (defmacro seq [param #* seq-code]

are what I mean by "mass syntax changes". The edits to the body of lambda-list that I mentioned, which are an example of a less trivial change, are the addition

      (and (isinstance arg HyExpression) (in (first arg) headers))
      (do (setv header (first arg))
          (assoc sections header [])
          ;; Don't use a header more than once. It's the compiler's problem.
          (.remove headers header)
          (.append (get sections header) arg))

Put the mass syntax changes in a different commit from nontrivial changes so I know what to read.

[allison-casey]

Oh right, I forgot there were changes to certain macro parsing logic. I think this addresses everything. I'll rebase off master to get the py3.10 fix in pprint tomorrow

[allison-casey]

I'll rebase off master to get the py3.10 fix in pprint tomorrow

Done

[Kodiologist]

Excellent work. Now I have to update all my own code (oof).