Add space around "->" to make source pep8 friendly #170
Conversation
rvprasad
force-pushed
the
master
branch
2 times, most recently
from
3f021f6
to
60f149c
Compare
Corresponding test also checks type annotation for parameters and assignment are pep8 friendly
There was a problem hiding this comment.
Thanks for the PR! We are already doing some random PEP 8 formatting in the code generator, so I think this is a reasonable PR. Could you please add a changelog entry in docs/changelog.rst? You can add a new 0.9.0 section. Example format:
0.9.0 - in development
----------------------
Bug fixes
~~~~~~~~~
* Description of your PR.
(Contributed by Venkatesh-Prasad Ranganath in <PR number and link here>.)
|
@berkerpeksag We already started a 0.9 section in #160 though it still waits your review. |
rvprasad
force-pushed
the
master
branch
3 times, most recently
from
e18f7b8
to
a08be12
Compare
|
@berkerpeksag I have added a new section to the |
docs/changelog.rst
Outdated
| @@ -2,6 +2,18 @@ | |||
| Release Notes | |||
| ============= | |||
|
|
|||
| 0.9.0 - in development | |||
| -------------------- | |||
There was a problem hiding this comment.
Nit: Add two missing - here.
There was a problem hiding this comment.
Fixed in rvprasad/astor@78e7c60a0.
docs/changelog.rst
Outdated
| 0.9.0 - in development | ||
| -------------------- | ||
|
|
||
| Bug Fixes |
There was a problem hiding this comment.
Fixed in rvprasad/astor@78e7c60a0. BTW, since "bug fix" is a compound noun, I think "Bug Fixes" is the correct usage.
docs/changelog.rst
Outdated
| ~~~~~~~~~ | ||
|
|
||
| * Change formatting of function and assignment type annotations to be more | ||
| PEP8 friendly. |
There was a problem hiding this comment.
Fixed in rvprasad/astor@78e7c60a0.
tests/test_code_gen.py
Outdated
| @@ -611,6 +611,24 @@ def test_annassign(self): | |||
| """ | |||
| self.assertAstRoundtripsGtVer(source, (3, 6)) | |||
|
|
|||
| @unittest.skipUnless(sys.version_info >= (3, 6), | |||
| "typing and ann assignment was introduced in " | |||
There was a problem hiding this comment.
Is "ann" short for "annotation" here? If so, it would be nice to use the longer version.
There was a problem hiding this comment.
I guess it is named after the AnnAssign node.
There was a problem hiding this comment.
"ann" was short for "annotated". Fixed in rvprasad/astor@78e7c60a0.
tests/test_code_gen.py
Outdated
| i: str = '3' | ||
| return i | ||
| """ | ||
| trgtxt = canonical(target) |
There was a problem hiding this comment.
Nit: We can reuse target here:
target = canonical(target)There was a problem hiding this comment.
Fixed in rvprasad/astor@78e7c60a0 by apply canonical to the literal before initializing target.
tests/test_code_gen.py
Outdated
| i : str = '3' | ||
| return i | ||
| """ | ||
| srctxt = canonical(source) |
There was a problem hiding this comment.
Nit: We can reuse source here:
source = canonical(source)There was a problem hiding this comment.
Fixed in rvprasad/astor@78e7c60a0 by apply canonical to the literal before initializing source.
tests/test_code_gen.py
Outdated
| return i | ||
| """ | ||
| trgtxt = canonical(target) | ||
| self.assertEqual(self.to_source(ast.parse(srctxt)).rstrip(), trgtxt) |
There was a problem hiding this comment.
More importantly, can't we directly use assertAstEqualsSource here?
There was a problem hiding this comment.
I have used assertAstEqualSource in rvprasad/astor@78e7c60a0.
Yes, it's on my TODO list. I still need to read the PEP first and am planning to work on it next month. |
To help the review process, I have enabled the "edits from maintainers". |
|
Thanks! |
Corresponding test also checks type annotation for parameters and assignment are pep8 friendly.