python / mypy

Optional static typing for Python
https://www.mypy-lang.org/
Other
18.56k stars 2.84k forks source link

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

Closed gvanrossum closed 8 years ago

gvanrossum commented 8 years ago

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 commented 8 years ago

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

gvanrossum commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

I'm not sure how to do that with Jukka's parser. (If it was Python's parser we could use the strategy for recognizing unfinished input used by code.py (based on codeop.py). But mypy's syntax for function annotations in comments is not valid Python.

Also, this might mask subtle syntax errors (a line with an unclosed paren followed by a line meant as a non-type comment).

I guess this should be a low priority feature. And we should carefully consider how to do it so that other tools interested in supporting this syntax can also parse it (e.g. PyCharm in 2.7 mode).

JukkaL commented 8 years ago

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 commented 8 years ago

Yeah, we probably need to augment PEP 484 with the comment-based function annotations anyways.

JukkaL commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

OK, let's go with 2.

gvanrossum commented 8 years ago

I've filed https://github.com/python/typing/issues/186 to track this as an addition to PEP 484.

gvanrossum commented 8 years ago

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

JukkaL commented 8 years ago

Agreed that this is important. On Tue, Mar 22, 2016 at 20:06 Guido van Rossum notifications@github.com 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 https://github.com/python/mypy/issues/1102#issuecomment-199996395

ddfisher commented 8 years ago

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 commented 8 years ago

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 notifications@github.com 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 https://github.com/python/mypy/issues/1102#issuecomment-200017429

gvanrossum commented 8 years ago

I expect it'll be easy enough in the current parser actually, and it looks like the new parser isn't going to be ready really soon (esp. for 2.7).

gvanrossum commented 8 years ago

The new parser is not in the 0.3.2 milestone (and IMO this is correct).

johnthagen commented 8 years ago

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 commented 8 years ago

Yes, it's # type: () -> None. What's unclear about that?

johnthagen commented 8 years ago

@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 commented 8 years ago

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