Complete the tokenize module type hints #984

mjpieters · 2017-03-12T01:47:19Z

Note that I used the 3.6 syntax for the TokenInfo namedtuple; @ambv stated he plans to move all of typeshed to that syntax.

JelleZijlstra · 2017-03-12T15:09:14Z

Can you fix the Travis failures? Looks like mypy doesn't actually support variable annotation syntax in pre-3.6 stubs.

ilevkivskyi · 2017-03-12T15:24:10Z

@ambv Is it true that you plan to move all stubs to 3.6 variable annotations?
I am asking because I have a small tool that does this automatically (the interface is still a bit "raw", but otherwise it works very well, preserves formatting, removes ellipses if necessary, etc.) see https://github.com/ilevkivskyi/com2ann.

It was out my focus for few months, but I am going to improve it and add opposite transformation (annotation to comments), support for function type comments (2.7 syntax), support for docstrings etc.

gvanrossum · 2017-03-12T19:14:47Z

Even if we were in agreement that we should use the 3.6 notation for all stubs I still would not want to do a mass conversion. Instead I would just do it opportunistically over time.

gvanrossum · 2017-03-14T18:54:32Z

I am a bit mystified by the test failures. I can repro these too and there seem to be two sets:

When running with --python-version 3.6 we run into a limitation on NamedTuple subclasses (they currently can't have properties or methods -- IIRC @ilevkivskyi is working on a fix?)
Also there's a missing import of Optional.
With earlier Python 3 versions we get "stdlib/3/tokenize.pyi:16: error: Variable annotation syntax is only supported in Python 3.6 and greater" -- this seems to indicate that mypy actually still checks stubs using an earlier Python version despite Default to type checking in Python 3.6 mode mypy#2915.

ilevkivskyi · 2017-03-14T19:06:23Z

@gvanrossum

they currently can't have properties or methods -- IIRC @ilevkivskyi is working on a fix?

I only did the runtime implementation in typing. I thought @JelleZijlstra wanted to try this
python/mypy#2719 (comment) after python/mypy#2719 is merged (the latter is ready if I remember correctly).

JelleZijlstra · 2017-03-14T19:07:54Z

Sure, I can work on that once python/mypy#2719 is in mypy.

gvanrossum · 2017-03-14T22:16:31Z

So now that python/mypy#3000 has landed, there are two remaining issues:

Missing import of Optional
We still get "NamedTuple class syntax is only supported in Python 3.6". That may be caused by the way that the typeshed tests pass the stub files on the command line?
The property definition isn't supported.

mjpieters · 2017-03-15T06:04:14Z

The property definition isn't supported.

Would an instance variable annotation work? It'd communicate what type to expect when accessing the attribute:

class TokenInfo(NamedTuple):
    # ...
    # @property
    exact_type: int

Otherwise, is the next option to retain exact_type as a method (so drop the @property decorator), or drop exact_type from the stub altogether?

JelleZijlstra · 2017-03-15T06:11:21Z

stdlib/3/tokenize.pyi

-def untokenize(iterable): ...
-def detect_encoding(readline): ...
-def tokenize(readline): ...
+def untokenize(iterable: Iterable[_Token]) -> Union[str, bytes]: ...


We should almost never return a Union because Union handling is often buggy in mypy and using Unions forces calling code into tedious isinstance checks. I think this one should just return Any.

Ah, no, we should use AnyStr.

AnyStr doesn't make sense here because it's a type variable; see Jukka's discussion in #871.

Ah, good point. The caller will only get str if they pass in an encoding, at which point they shouldn't have to deal with bytes. Any it is.

JelleZijlstra · 2017-03-15T06:13:40Z

I think the best option would be to make the NamedTuple class private and inherit from that, something like this:

class _TokenInfo(typing.NamedTuple):
    encoding: str

class TokenInfo(_TokenInfo):
    @property
    def exact_type(self) -> int: ...

I haven't tested it but I believe mypy supports this. It's also consistent with the way this class is actually implemented in tokenize.py.

gvanrossum · 2017-03-15T15:08:39Z

It's a NamedTuple -- a plain attribute sounds like a fine idea to me.

JelleZijlstra · 2017-03-15T15:17:59Z

If we make it a plain attribute, type checkers will think that you need to pass in six arguments (including exact_type) in order to construct a TokenInfo, but you really only need five.

gvanrossum · 2017-03-15T16:13:53Z

Fair enough, so the solution with the helper class should work. (We also should fix this in mypy so it won't be necessary.) But there's still the issue of the PEP 526 syntax not being supported (yet?) by mypy in stubs for Python 3.2--3.5. If we're not fixing that we should just cave and use the classic NamedTuple syntax.

…

On Wed, Mar 15, 2017 at 8:18 AM, Jelle Zijlstra ***@***.***> wrote: If we make it a plain attribute, type checkers will think that you need to pass in six arguments (including exact_type) in order to construct a TokenInfo, but you really only need five. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#984 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACwrMnknn7Ycfg6H9BK-C1BmyzrMDrZkks5rmAEogaJpZM4MaZx1> .

-- --Guido van Rossum (python.org/~guido)

mjpieters · 2017-03-15T16:33:37Z

But there's still the issue of the PEP 526 syntax not being supported
(yet?) by mypy in stubs for Python 3.2--3.5. If we're not fixing that we
should just cave and use the classic NamedTuple syntax.

I'll cave and use the 3.5 style for now. @ambv can upgrade it to 3.6 if and when support is there.

…use Any

ambv · 2017-03-15T16:56:49Z

stdlib/3/tokenize.pyi

-def tokenize(readline): ...
+def untokenize(iterable: Iterable[_Token]) -> Any: ...
+def detect_encoding(readline: Callable[[], bytes]) -> Tuple[str, Sequence[bytes]]: ...
+def tokenize(readline: Callable[[], bytes]) -> Generator[TokenInfo, None, None]: ...


@mjpieters This is fine but in the future, if you're type hinting the return value of a simple generator, simply use -> Iterator[SomeType].

mjpieters · 2017-03-15T16:58:11Z

stdlib/3/tokenize.pyi

-ENCODING = 0
+COMMENT = ...  # type: int
+NL = ...  # type: int
+ENCODING = ...  # type: int


You'll want to update token.py too in that case; I followed the style from that stub here.

Might be good to add this to CONTRIBUTING.md or the linter; I've been unsure in the past what style to use for this kind of thing.

gvanrossum · 2017-03-15T16:58:37Z

Whee! Thanks everyone for the truly multidisciplinary review and improvements.

ambv · 2017-03-21T03:30:34Z

If this is not clear from the comments below:

stubs can now use the PEP 526 syntax for variables and fields after Always use the default Python 3 version to parse stub files mypy#3000 was merged
but you cannot use typing features from 3.6 if --python-version is set to something lower

In other words, if we change variables in tokenize.pyi to use PEP 526 annotations, everything works fine when running with --python-version 3.5. But, if you use the 3.6 NamedTuple syntax, you'll get:

error: NamedTuple class syntax is only supported in Python 3.6

gvanrossum · 2017-03-21T03:58:43Z

Aha! I had missed the distinction. If you think this is worth fixing please file a mypy bug!

Complete the tokenize module type hints

868a824

gvanrossum mentioned this pull request Mar 14, 2017

Default to type checking in Python 3.6 mode python/mypy#2915

Merged

gvanrossum mentioned this pull request Mar 14, 2017

Always use the default Python 3 version to parse stub files python/mypy#3000

Merged

gvanrossum added the blocked label Mar 14, 2017

Add missing import for Optional

7b45b29

JelleZijlstra requested changes Mar 15, 2017

View reviewed changes

Martijn Pieters and others added 2 commits March 15, 2017 09:49

Use a 3.5-style named tuple, untokenize speaks with forked tongue so …

9b630ca

…use Any

Use explicit types

582478c

ambv reviewed Mar 15, 2017

View reviewed changes

ambv merged commit 3f0eb99 into python:master Mar 15, 2017

mjpieters commented Mar 15, 2017

View reviewed changes

ambv removed the blocked label Mar 15, 2017

mjpieters deleted the tokenize_update branch March 15, 2017 17:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Complete the tokenize module type hints #984

Complete the tokenize module type hints #984

mjpieters commented Mar 12, 2017

JelleZijlstra commented Mar 12, 2017

ilevkivskyi commented Mar 12, 2017 •

edited

Loading

gvanrossum commented Mar 12, 2017

gvanrossum commented Mar 14, 2017 •

edited

Loading

ilevkivskyi commented Mar 14, 2017

JelleZijlstra commented Mar 14, 2017 •

edited by gvanrossum

Loading

gvanrossum commented Mar 14, 2017

mjpieters commented Mar 15, 2017 •

edited

Loading

JelleZijlstra Mar 15, 2017

mjpieters Mar 15, 2017

JelleZijlstra Mar 15, 2017

mjpieters Mar 15, 2017

JelleZijlstra commented Mar 15, 2017

gvanrossum commented Mar 15, 2017

JelleZijlstra commented Mar 15, 2017

gvanrossum commented Mar 15, 2017 via email

mjpieters commented Mar 15, 2017

ambv Mar 15, 2017

mjpieters Mar 15, 2017

ambv Mar 15, 2017

JelleZijlstra Mar 15, 2017

gvanrossum commented Mar 15, 2017

ambv commented Mar 21, 2017 •

edited

Loading

gvanrossum commented Mar 21, 2017 via email

Complete the tokenize module type hints #984

Complete the tokenize module type hints #984

Conversation

mjpieters commented Mar 12, 2017

JelleZijlstra commented Mar 12, 2017

ilevkivskyi commented Mar 12, 2017 • edited Loading

gvanrossum commented Mar 12, 2017

gvanrossum commented Mar 14, 2017 • edited Loading

ilevkivskyi commented Mar 14, 2017

JelleZijlstra commented Mar 14, 2017 • edited by gvanrossum Loading

gvanrossum commented Mar 14, 2017

mjpieters commented Mar 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JelleZijlstra commented Mar 15, 2017

gvanrossum commented Mar 15, 2017

JelleZijlstra commented Mar 15, 2017

gvanrossum commented Mar 15, 2017 via email

mjpieters commented Mar 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gvanrossum commented Mar 15, 2017

ambv commented Mar 21, 2017 • edited Loading

gvanrossum commented Mar 21, 2017 via email

ilevkivskyi commented Mar 12, 2017 •

edited

Loading

gvanrossum commented Mar 14, 2017 •

edited

Loading

JelleZijlstra commented Mar 14, 2017 •

edited by gvanrossum

Loading

mjpieters commented Mar 15, 2017 •

edited

Loading

ambv commented Mar 21, 2017 •

edited

Loading