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

add support for typed function parameters #140

Closed

Conversation

@scottbelden
Copy link
Contributor

scottbelden commented Sep 11, 2018

Resolves #127

Please let me know what you think.

This should allow for typed parameters like def foo(x: int) and def foo(x: List[int]). I didn't try testing when parameters like *args or **kwargs are typed. I think that would be good to add in the future but it just wasn't as necessary for me at the moment.

This does not add support for typed variables or function return types, but I hope to add support for the later in the near future assuming this is accepted.

@scottbelden

This comment has been minimized.

Copy link
Contributor Author

scottbelden commented Sep 11, 2018

Also, when type annotations are present the type of the object is typed_name instead of name. In redbaron I would assumed TypedName would be a subclass of Name but I haven't looked over there yet to see if this makes sense.

@Psycojoker

This comment has been minimized.

Copy link
Member

Psycojoker commented Sep 23, 2018

Hello,

Just to inform you that I've started looking at your PR, but it takes me /hours/ to correctly review most of the new grammar additions to baron (it's super sensible and I had quite some bad surprise) and this one is pretty complex.

If you haven't looked at it, this might helps you https://baron.readthedocs.io/en/latest/grammar.html#typed-arguments

Anyway, thanks a lot for this PR :)

@scottbelden

This comment has been minimized.

Copy link
Contributor Author

scottbelden commented Sep 24, 2018

Okay. Let me know if there is anything I can do to make the PR more bite size and easier to review.

@Psycojoker

This comment has been minimized.

Copy link
Member

Psycojoker commented Oct 8, 2018

Finally merged (by hand so github don't detect that), thx again for your contribution :)

I end up adding more tests and finished all the cases (*a: int, **a: int), took me way more time to understand the grammar diff since there was a refactoring in python grammar in the middle of that for some reason.

@Psycojoker Psycojoker closed this Oct 8, 2018
@scottbelden

This comment has been minimized.

Copy link
Contributor Author

scottbelden commented Oct 8, 2018

Thanks!

@JosXa

This comment has been minimized.

Copy link

JosXa commented Oct 24, 2018

Hey awesome, I just stumbled over the problem and immediately I find out that there is a patch in the works on GitHub... nice work!

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

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.