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

Alternative to @v_args() #1468

Open
makukha opened this issue Sep 12, 2024 · 2 comments
Open

Alternative to @v_args() #1468

makukha opened this issue Sep 12, 2024 · 2 comments

Comments

@makukha
Copy link

makukha commented Sep 12, 2024

Suggestion

  1. Use explicitly named decorators: @wrap_every (class level only), @inline_children (method level only)
  2. Let Transformer decide which arguments to pass based on method signature:
    • If method has single argument, pass children regardless of its name (this will preserve backwards compatibility)
    • If args are subset of {meta, tree, children} — pass respective values.
  3. v_args is left unchanged for backwards compatibility.

This may make API more explicit and sound. The second approach is used in e.g. Pydantic (see validators in v.1).

I'm not aware of all possible use cases for v_args, but at least some of them are covered.

Describe alternatives you've considered
@v_args is the only alternative.

Additional context
Usage examples:

Class level only @wrap_every

@wrap_every(inline_children)
class Demo(Transformer):
    def rule(self, *items): ...

Method level only @inline_children

class Demo(Transformer):
    def rule1(self, children): ...
    @inline_children
    def rule2(self, *items): ...

Method signature auto detection

class Demo(Transformer):
    def rule1(self, items): ...  # children passed
    def rule2(self, tree): ...  # tree passed
    def rule3(self, meta, children): ...  # pass meta, children
    def rule4(self, meta, items): ...  # to be decided: raise error or pass children to items
    @inline_children
    def rule5(self, tree, meta, *items): ...

If this change sounds reasonable, I'm ready to work on PR.

@erezsh
Copy link
Member

erezsh commented Sep 12, 2024

Use explicitly named decorators: @wrap_every (class level only), @inline_children (method level only)

Can you explain your rationale?

@makukha
Copy link
Author

makukha commented Sep 13, 2024

Use explicitly named decorators: @wrap_every (class level only), @inline_children (method level only)

Can you explain your rationale?

First of all, let me say that Lark is an awesome project and already saved me days of life. I appreciate hard work, dedication and talent of Lark's maintainers. I don't understand much in parsers and lexers, but want to dedicate some of my time to the field where I touch Lark the most as a developer: its API.

The main reasons for change proposed are

  1. Separation of features applicable in different contexts.
  2. Easier support: every feature can be updated/fixed independently.
  3. Readability, better naming, less user code (lower cognitive load).

For v_args(meta=, tree=):

  • This syntax can be eliminated, args can be deduced from method signature (methods are not called manually, Lark can do this itself).

For v_args(wrapper=):

  • This feature does not make sense when applied to individual method, is applicable only at class level.

For v_args(inline):

  • Not immediately obvious what is inlined, inline_children seems better to me. Also this will be more readable when used as a standalone decorator on methods.
  • Actually, it can be used at class level too, @inline_children is more readable that @wraps_every(inline_children), if your question was about this case.

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

No branches or pull requests

2 participants