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

Support multi-line # type: comments for functions #1102

Closed
gvanrossum opened this issue Jan 8, 2016 · 22 comments
Closed

Support multi-line # type: comments for functions #1102

gvanrossum opened this issue Jan 8, 2016 · 22 comments

Comments

@gvanrossum
Copy link
Member

Some linters get mad about long lines, even comments, so we would like to be able to break long # type: comments into multiple lines. This is especially desirable for function annotations if the function has lots of arguments, since you can't shorten those using type aliases. (And claiming that's just bad code doesn't do anyone any good -- I know, but sometimes I can't afford to fix it.)

@JukkaL JukkaL added the feature label Jan 8, 2016
@JukkaL
Copy link
Collaborator

JukkaL commented Jan 8, 2016

Yes. I've actually been waiting for somebody to complain about this :-)

@gvanrossum
Copy link
Member Author

But what should it look like? # type: on every line, or just on the first line? If the latter, we could get in trouble trying to parse a # type: comment followed by an ordinary comment. So I propose the former:

def whatever(arg1, arg2, arg3,
             arg5, arg5, arg6):
    # type: (int, int, int,
    # type:  str, str, str) -> float
    # this is not a type comment
    return (arg1 + arg2 + arg3 +
            float(arg4) + float(arg5) + float(arg6))

@refi64
Copy link
Contributor

refi64 commented Jan 8, 2016

What about only considering the next line to be type comment if it simply isn't complete? e.g. (a, isn't finished (no closing paren), so the next line would be considered the rest. Same this for a ->, a[b, etc.

@gvanrossum
Copy link
Member Author

gvanrossum commented Jan 8, 2016 via email

@JukkaL
Copy link
Collaborator

JukkaL commented Jan 8, 2016

I always assumed that we'd keep track of open parens and square brackets to determine when the type annotation ends. Example:

def whatever(arg1, arg2, arg3,
             arg5, arg5, arg6):
    # type: (int, int, int,
    #        str, str, str) -> float
    # this is not a type comment
    return (arg1 + arg2 + arg3 +
            float(arg4) + float(arg5) + float(arg6))

Reporting parse errors correctly might be a little tricky. Mypy should perhaps explicitly mention in a parse error message whether the error is within an (assumed) type annotation.

However, repeating # type: would also work and would probably be a little easier to implement. I don't have a strong opinion on this, though the single # type: alternative looks a little cleaner to me.

Should we discuss this in https://github.com/ambv/typehinting as well?

@gvanrossum
Copy link
Member Author

gvanrossum commented Jan 8, 2016 via email

@JukkaL
Copy link
Collaborator

JukkaL commented Feb 8, 2016

Another idea is to support optionally including argument names in comments:

def whatever(arg1, arg2, arg3,
             arg4, arg5, arg6):
    # type: (arg1: int, arg2: int, arg3: int,
    #        arg4: str, arg5: str, arg6: str) -> float
    # this is not a type comment
    return (arg1 + arg2 + arg3 +
            float(arg4) + float(arg5) + float(arg6))

This way it would be easier to map argument names to types, especially if there are a ton of arguments. The obvious tradeoff is that there would be more redundancy, but mypy could check that the argument names match, and we could have a tool that automatically inserts dummy annotations with the correct argument names.

Another idea for specifying signatures of functions with many arguments:

def whatever(
         arg1,  # type: int 
         arg2,  # type: int
         arg3,  # type: int
         arg4,  # type: str
         arg5,  # type: str
         arg6,  # type: str
         ):
    # type: (...) -> float
    # this is not a type comment (note that "..." above is literal)
    return (arg1 + arg2 + arg3 +
            float(arg4) + float(arg5) + float(arg6))

This way argument name and type would closer to each other, a bit like in the Python 3 annotation syntax. The tradeoff is that there can only be a single argument per line. For functions with only a few arguments the original annotation syntax is fine and we'll want to preserve it as the common case option.

@gvanrossum
Copy link
Member Author

I'd like to hear from the PyCharm developers -- @vlasovskikh do you have any preference? How easy would any of these be to add to PyCharm?

@vlasovskikh
Copy link
Member

Either form would be relatively easy to support in PyCharm.

There are a couple of minor issues with the first named-arguments-in-comments form. Our code formatter can automatically reformat the second form (with #type: ...) according to the project code style, while the first one would require a special formatter for this custom multi-line syntax. For the first form we would have update named references as a part of the rename parameter refactoring while the second form doesn't require that.

@ddfisher
Copy link
Collaborator

ddfisher commented Mar 2, 2016

The second form seems preferable to me. My understanding is that good style generally dictates having one arg per line once you need to wrap arguments anyway, so it's a pretty natural fit.

@gvanrossum
Copy link
Member Author

OK, let's go with 2.

@gvanrossum
Copy link
Member Author

I've filed python/typing#186 to track this as an addition to PEP 484.

@JukkaL JukkaL changed the title Support continuation lines in long # type: comments Support multi-line # type: comments for functions Mar 18, 2016
@gvanrossum
Copy link
Member Author

This is settled in the PEP. It's pretty important for our internal customers. Maybe move to 0.3.2?

@JukkaL
Copy link
Collaborator

JukkaL commented Mar 22, 2016

Agreed that this is important.
On Tue, Mar 22, 2016 at 20:06 Guido van Rossum [email protected]
wrote:

This is settled in the PEP. It's pretty important for our internal
customers. Maybe move to 0.3.2?


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#1102 (comment)

@ddfisher
Copy link
Collaborator

I think the main question for me here is: do we want to implement this in the current parser or wait for the new one?

@JukkaL
Copy link
Collaborator

JukkaL commented Mar 22, 2016

Is the new Python 2 parser going to be in 0.3.2? If not, we probably should
implement it in the current parser. If yes, we could go either way I guess.

On Tue, Mar 22, 2016 at 20:46 David Fisher [email protected] wrote:

I think the main question for me here is: do we want to implement this in
the current parser?


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#1102 (comment)

@gvanrossum
Copy link
Member Author

gvanrossum commented Mar 22, 2016 via email

@gvanrossum
Copy link
Member Author

gvanrossum commented Mar 22, 2016 via email

@johnthagen
Copy link
Contributor

PEP484 doesn't mention an explicit example of how to define return types for functions with no parameters. In Python 3, there would be no annotations at all.

It mentions that you can use ellipsis instead of type hinting an argument list, but what if there are no arguments? Sorry if this is a nitpicky question.

It would assume this is the valid way:

def fun():
    # type: (...) -> None
    pass

But when I first got going, I thought perhaps this was (mimicking what I would expect from Python 3):

def fun():
    # type: () -> None
    pass

Just some feedback from a first time user of the new syntax and trying to get things to work in PyCharm. Both of the above forms currently work in PyCharm 2016.1RC.

@gvanrossum
Copy link
Member Author

gvanrossum commented Mar 22, 2016 via email

@johnthagen
Copy link
Contributor

@gvanrossum For me, it was just because there wasn't an explicit example for the empty case, but there were two that showed how to use the # type: (...) -> format for two different purposes, so it made me think perhaps it was also used for the empty case as well.

I'd like to say I would be the only one to make this mistake, but having made it I'd suggest adding a brief example or note in the PEP. If not, though, not a big deal. Thanks for the clarification.

@ddfisher
Copy link
Collaborator

ddfisher commented Apr 8, 2016

It's looking like the new parser will be in 0.3.2 now. I'll implement this after it lands (see #1353).

@gvanrossum gvanrossum modified the milestones: 0.4.x, 0.4.2 May 19, 2016
@JukkaL JukkaL removed the priority label Jun 7, 2016
@gvanrossum gvanrossum modified the milestones: 0.4.x, 0.4.2 Jun 9, 2016
gvanrossum pushed a commit that referenced this issue 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 join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants