Python3 annotations #74

bluebird75 · 2018-07-07T19:54:08Z

Implementation for #4 is done.

All tests are ported and pass successfully.

A few remarks on the implementation :

command-line options are the same as mypy : -2, --py2, -3, --py3, --python-version
the code will not annotate functions with an existing py2 or py3 annotation
I ignored the concept of long form for python 3 annotations. Function with many arguments are still annotated inline.
test_annotate.py and test_annotate_json.py have been renamed with a _py2 and _py3 suffix depending on what they are testing.

Looking forward for your feedback.

… return values annotations are supported so far.

…ut default values. No support for * or ** arguments

…, -3, -py3, --python-version

…tyle verification

gvanrossum · 2018-07-09T00:37:38Z

Thanks! Reviewing this will take a bit of time, but your work is very much appreciated already!

bluebird75 · 2018-07-09T09:21:23Z

Sure. I'll be patient then...

gvanrossum

Here's my initial review. It seems to work great!

gvanrossum · 2018-07-31T17:43:42Z

pyannotate_tools/annotations/__main__.py

+parser.add_argument('--py2', '-2', action='store_true',
+                    help='''Annotate for Python 2 with comments (default)''')
+parser.add_argument('--py3', '-3', action='store_true',
+                    help='''Annotate for Python 3 with argument and return value annotations''')


Arguably you could implement the latter two as action='store_const', dest='python_version', const='2' (or '3').

gvanrossum · 2018-07-31T17:45:11Z

pyannotate_tools/annotations/__main__.py

+        sys.exit('--python-version must be 2 or 3')
+
+    if (args.py2 and args.py3) or (args.py2 and args.python_version) or (args.py3 and args.python_version):
+        sys.exit('You can not use multiple annotation version specifier')


If you implement my suggestion above I would skip this check -- I would just let the last flag win (there are scenarios where that's actually useful).

gvanrossum · 2018-07-31T17:47:24Z

pyannotate_tools/annotations/__main__.py

    except OSError as err:
        sys.exit("Can't open type info file: %s" % err)

+    if args.python_version not in (None, '2','3'):


Add a space after the second comma. (And if you add a default you won't have to check for None.)

gvanrossum · 2018-07-31T17:48:05Z

pyannotate_tools/annotations/__main__.py

                    help="Only annotate functions with trivial types")
+parser.add_argument('--python-version', action='store',
+                    help='''Choose annotation style, 2 for Python 2 with comments (the 
+                         default), 3 for Python 3 with direct annotation''' )


Add default='2'. Then you won't have to check for None.

gvanrossum · 2018-07-31T17:50:14Z

pyannotate_tools/fixes/fix_annotate.py

-into
+into:
+
+- with options {'annotation_style'='py2'} :


I'd use a comment as part of the code here (for a second or two I thought you were showing a with statement here).

gvanrossum · 2018-07-31T18:01:32Z

pyannotate_tools/fixes/fix_annotate.py

+        # Also add 'from typing import Any' at the top if needed.
+        self.patch_imports(argtypes + [restype], node)
+
+


There should only be a single blank line between two methods.

gvanrossum · 2018-07-31T18:16:37Z

pyannotate_tools/fixes/fix_annotate.py

        # Insert '# type: {annot}' comment.
        # For reference, see lib2to3/fixes/fix_tuple_params.py in stdlib.
        if len(children) >= 1 and children[0].type != token.NEWLINE:
+                # one liner function


This comment feels misindented.

gvanrossum · 2018-07-31T18:20:03Z

pyannotate_tools/fixes/fix_annotate.py

+            it = iter([])
+        elif len(args.children) == 0:
+            # function with 1 argument
+            it = iter( [ args ] )


This doesn't follow PEP 8 -- it should be iter([args]).

gvanrossum · 2018-07-31T18:20:22Z

pyannotate_tools/fixes/fix_annotate.py

+
+        rpar.changed()
+
+    def add_py2_annot( self, argtypes, restype, node, results):


No space after the (.

gvanrossum · 2018-08-03T18:54:33Z

@bluebird75 I realize I took my time reviewing this. I am going on vacation for the next 3 weeks so unless I get terribly bored at my holiday destination I won't be reviewing your next commit very soon. Sorry! It's good work, it just needs one more round.

bluebird75 · 2018-08-04T09:46:44Z

So, to sum-up, a few PEP8 issues and better use of argparse. That's not too bad. I was never very PEP8 friendly so that's not so surprising. :-) I'll work on all that in the coming weeks and provide an update.

You probably don't know but we've met before: I interviewed you at FOSDEM 2002 in Brussel when you came to receive the FSF award: http://www.freehackers.org/Fosdem_2002:_Guido_van_Rossum_interview . I can't believe it's been 16 years !

bluebird75 · 2018-08-26T21:14:09Z

Here is a new version. Note that the CI failure is not due to the changes here but to some changes at Travis CI. See #76 for fixing them.

gvanrossum

Almost ready to merge! Thanks again for your patience.

gvanrossum · 2018-08-27T17:05:01Z

pyannotate_tools/annotations/__main__.py

 parser.add_argument('-s', '--only-simple', action='store_true',
                    help="Only annotate functions with trivial types")
+parser.add_argument('--python-version', action='store', default='2',
+                    help='''Choose annotation style, 2 for Python 2 with comments (the 


The '''...''' thing seems cute but looks weird (I've never seen it before). While it seemingly works, I prefer using "..." for the help strings, and if they don't fit on the line, just use another "..." string on the next line (but make sure there's a space at the end of one or at the beginning of the other). The ones below should definitely switch back to "...".

gvanrossum · 2018-08-27T17:22:18Z

pyannotate_tools/fixes/fix_annotate.py

-into
+into a type annoted version:
+
+	  def foo(self, bar, baz=12):


Spaces around the = -- that's what the code actually does, and that's what PEP 8 requires. Ditto below.

Latest remarks are resolved now.

gvanrossum · 2018-08-31T14:56:11Z

And there was great rejoicing!

gvanrossum · 2018-08-31T15:12:42Z

Seriously, thanks for hanging in there and shaving some yaks. We should celebrate by doing a point release!

bluebird75 · 2018-08-31T20:48:29Z

I'll be more than happy to celebrate with a release. I'll see where I can contribute next. Multi-source type annotations looks like a big topic to me.

laike9m · 2018-11-11T12:29:05Z

Do we have a release plan for this?

gvanrossum · 2018-11-13T21:23:34Z

It's released now: https://pypi.org/project/pyannotate/1.0.7/

as of v1.0.7, merged in dropbox#74

as of v1.0.7, merged in #74

bluebird75 and others added 28 commits June 23, 2018 08:37

Fix exchanged pause() and resume() in phase 1

414d36a

Replace resume()/pause() by start()/stop()

f9e973e

Replace resume()/pause() by start()/stop()

5aea713

Ignore non relevante files

96add09

Add a script to run all tests run in CI

8b28f8c

Add a TODO

db8c876

Add dedicated python 3 annotations refactorer and related tests. Only…

e21f333

… return values annotations are supported so far.

Add my playground file

227e52f

Add annotations to functions with positional arguments, with or witho…

37ef714

…ut default values. No support for * or ** arguments

Fix local test executer

fa4a06e

Enable 8 more tests

1449ebc

Do not annotate self/cls for static/instance methods

6c9b8ea

Add support for *nargs and **kwargs

970dbd4

Do not perform any special treatment for long lines annotations.

6b8d8a7

Merge remote-tracking branch 'origin/dropbox_merge'

7b80589

Update TODO

2fa4480

Add new command line arguments to control annotation style : -2, -py2…

8f032a2

…, -3, -py3, --python-version

Switch between python 2 and python 3 style annotations at run-time

eb669c2

Adjust annotest_json tests to Python 3 annotations

fdb8d2a

Skip functions with python3 style annotation

981ffe7

Put each check in a dedicated function

052acb0

Add ending _py2 or _py3 to test names depending on their annotation s…

1a83ffc

…tyle verification

Put spaces around the equal sign for default arguments

959df0b

Update TODO

0eb2274

Rename TODO.txt to TODO.md

0b5d0db

Merge branch 'master' of https://github.com/bluebird75/pyannotate

f0d6821

Update TODO.md

fe6f6e2

Merge branch 'master' into python3-annotations

647bfe5

gvanrossum reviewed Jul 31, 2018

View reviewed changes

bluebird75 added 3 commits August 22, 2018 23:04

Simplify command line parsing for annotation style version

5249f17

PEP8 fixes

9286a9a

Improve the comment text.

6ce0839

gvanrossum reviewed Aug 27, 2018

View reviewed changes

bluebird75 added 2 commits August 31, 2018 16:08

Fix per PEP8 recommendations

9d928f7

Use single quote for help text

5bbb1ac

gvanrossum merged commit ad82c01 into dropbox:master Aug 31, 2018

carver added a commit to carver/pyannotate that referenced this pull request Jun 11, 2019

docs: note that py3 annotations are available

8161cec

as of v1.0.7, merged in dropbox#74

carver mentioned this pull request Jun 11, 2019

Readme: note that py3 annotations are available #86

Merged

gvanrossum pushed a commit that referenced this pull request Jun 11, 2019

docs: note that py3 annotations are available (#86)

f06705d

as of v1.0.7, merged in #74

		# Also add 'from typing import Any' at the top if needed.
		self.patch_imports(argtypes + [restype], node)


		rpar.changed()

		def add_py2_annot( self, argtypes, restype, node, results):

Python3 annotations #74

Python3 annotations #74

Uh oh!

Conversation

bluebird75 commented Jul 7, 2018

Uh oh!

gvanrossum commented Jul 9, 2018 via email

Uh oh!

bluebird75 commented Jul 9, 2018

Uh oh!

gvanrossum left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gvanrossum commented Aug 3, 2018

Uh oh!

bluebird75 commented Aug 4, 2018

Uh oh!

bluebird75 commented Aug 26, 2018

Uh oh!

gvanrossum left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gvanrossum commented Aug 31, 2018

Uh oh!

gvanrossum commented Aug 31, 2018

Uh oh!

bluebird75 commented Aug 31, 2018

Uh oh!

laike9m commented Nov 11, 2018

Uh oh!

gvanrossum commented Nov 13, 2018

Uh oh!

Uh oh!