Skip to content
This repository has been archived by the owner on Jul 5, 2023. It is now read-only.

[2.7, 3.5] Support the per-argument function comment syntax #5

Merged
merged 7 commits into from
Jun 22, 2016

Conversation

ddfisher
Copy link
Collaborator

@ddfisher ddfisher commented Jun 16, 2016

For Python 2.7:
Adds a type_comments list of strings to the arguments object containing
the per-argument type comments of a function, if any.

For Python 3.5:
Puts per-argument type comments into the annotation slot of the
argument as a string expression.

This change contains some autogenerated files:

  • Python/graminit.c is autogenerated from Grammar/Grammar
  • Include/Python-ast.h and Python/Python-ast.c are autogenerated from Parser/Python.asdl

Adds a type_comments list of strings to the arguments object containing
the per-argument type comments of a function, if any.
Puts per-argument type comments into the annotation slot of the
argument as a string expression.
@ddfisher
Copy link
Collaborator Author

@gnprice take a look when you have time!

@gnprice
Copy link

gnprice commented Jun 21, 2016

Looking now! Eager to see this in. :-)

a `type_ignores` field which contains a list of lines which have been
`# type: ignored`.
type comment. Per-argument function comments are put into the annotation
field of each argument. `parse` has been augmented so it can parse
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This paragraph is probably best turned into bullets at this point -- the four sentences are fairly independent of each other, and the transition from "per-argument function comments" to "parse has been augmented" is particularly abrupt.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Changed.

@gnprice
Copy link

gnprice commented Jun 21, 2016

Done reading. Looks good! I have a number of comments above, but I think they are all small, and I think the only one that affects the behavior is about a syntax-error case.

Python 2 per-argument type comments are now converted into Python 3
annotations.

args = [convert_arg(arg) for arg in n.args]
def get_type_comment(i):
if i < len(n.type_comments):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, will this work if there are no type comments -- do we end up with an empty sequence for n.type_comments, or None?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an empty sequence.

@gnprice
Copy link

gnprice commented Jun 21, 2016

Done reading the new commit too.

@ddfisher ddfisher force-pushed the multiline-function-comments branch from 582aa47 to 314d17d Compare June 22, 2016 19:00
@ddfisher
Copy link
Collaborator Author

Pushed new commit with feedback.

- `parse` has been augmented so it can parse function signature types when
called with `mode=func_type`.
- `Module` has a `type_ignores` field which contains a list of
lines which have been `# type: ignored`.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, it looks like the "d" is on the wrong side of these backticks. A pre-existing nit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooh, let me fix that.

@gnprice
Copy link

gnprice commented Jun 22, 2016

Finished reading through the new version -- just a couple of comments which are all on documentation. (I wish I knew a reasonable way to get GitHub to tell me what the recent comments were.)

-- type_comments is used to support the per-argument type comment syntax.
-- It is either an empty list or a list with length equal to the number of
-- args (including varargs and kwargs, if present) and with members set to the
-- string of each arg's type comment, if present, or None otherwise.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this aligned funny, or is GitHub displaying it in a surprising way?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file uses tabs. Fixed

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. :)

@ddfisher
Copy link
Collaborator Author

Thanks for the detailed review!

@ddfisher ddfisher merged commit f29a0fb into master Jun 22, 2016
@ddfisher ddfisher deleted the multiline-function-comments branch June 22, 2016 23:17
@ddfisher
Copy link
Collaborator Author

PEP clarifications here: python/typing#237

gvanrossum pushed a commit to python/mypy that referenced this pull request Jun 23, 2016
This adds support for the (Python 2) per-argument type comment syntax now that python/typed_ast#5 has landed. It only works when using `--fast-parser`; without that these comments will be ignore.

The code works by converting Python 2.7 per-argument type comments to Python 3 annotations in the typed_ast's conversion module -- this allows them to be mixed with the # type: (...) -> None syntax.

Fixes #1102.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants