Skip to content

bpo-33475: Fix and improve converting annotations to strings.#6774

Merged
ambv merged 3 commits into
python:masterfrom
serhiy-storchaka:ast-unparse-omit-parens
May 17, 2018
Merged

bpo-33475: Fix and improve converting annotations to strings.#6774
ambv merged 3 commits into
python:masterfrom
serhiy-storchaka:ast-unparse-omit-parens

Conversation

@serhiy-storchaka

@serhiy-storchaka serhiy-storchaka commented May 12, 2018 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member
  • Fix miscellaneous bugs in converting annotations to strings.
  • Optimize parenthesis in the string representation.
  • Clean up the C code.

https://bugs.python.org/issue33475

@serhiy-storchaka
bpo-33475: Fix and improve converting annotations to strings.
33ab33b 
* Fix miscellaneous bugs in converting annotations to strings.
* Optimize parenthesis in the string representation.
* Clean up the C code.
@serhiy-storchaka serhiy-storchaka added type-bug An unexpected behavior, bug, or error needs backport to 3.7 labels May 12, 2018
@serhiy-storchaka serhiy-storchaka requested a review from ambv May 12, 2018 22:31
@gvanrossum

gvanrossum commented May 13, 2018

Copy link
Copy Markdown
Member

@ambv should review it

JelleZijlstra
JelleZijlstra reviewed May 13, 2018
View reviewed changes
Comment thread Misc/NEWS.d/next/Core and Builtins/2018-05-13-01-26-18.bpo-33475.rI0y1U.rst Outdated
@@ -0,0 +1,2 @@
Fixed miscellaneous bugs in onverting annotations to strings and optimize

@JelleZijlstra JelleZijlstra May 13, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

converting, optimized (to make tense consistent), parentheses (on the next line)

@ambv ambv merged commit 64fddc4 into python:master May 17, 2018
@miss-islington

miss-islington commented May 17, 2018

Copy link
Copy Markdown
Contributor

Thanks @serhiy-storchaka for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

@ambv

ambv commented May 17, 2018

Copy link
Copy Markdown
Contributor

Thanks for the PR, @serhiy-storchaka! I won't lie, I don't like the macros, I find them hiding actual control flow in the functions. But I won't argue with all the fixes you made.

@miss-islington

miss-islington commented May 17, 2018

Copy link
Copy Markdown
Contributor

Sorry, @serhiy-storchaka and @ambv, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 64fddc423fcbe90b8088446c63385ec0aaf3077c 3.7

@ambv

ambv commented May 17, 2018

Copy link
Copy Markdown
Contributor

Oops, looks like 3.7 needs help.

@serhiy-storchaka serhiy-storchaka deleted the ast-unparse-omit-parens branch May 17, 2018 06:25
@serhiy-storchaka

serhiy-storchaka commented May 17, 2018

Copy link
Copy Markdown
Member Author

Thank you for merging the PR and sorry for using macros. My reasons are that they make the code clearer to me they are already used everywhere in the AST processing. In any case this patch changed virtually every line of the code. Macros can help to avoid such large changes in future (if we will add a hidden parameter passed to most functions) or can help to automate changes because they uses are one-line and well searchable.

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request May 17, 2018
@serhiy-storchaka
[3.7] bpo-33475: Fix and improve converting annotations to strings. (p…
c5e760e 
…ythonGH-6774).

(cherry picked from commit 64fddc4)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@bedevere-bot

bedevere-bot commented May 17, 2018

Copy link
Copy Markdown

GH-6927 is a backport of this pull request to the 3.7 branch.

serhiy-storchaka added a commit that referenced this pull request May 20, 2018
@serhiy-storchaka
[3.7] bpo-33475: Fix and improve converting annotations to strings. (G…
b32f889 
…H-6774). (GH-6927)

(cherry picked from commit 64fddc4)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra left review comments

@ambv ambv Awaiting requested review from ambv

Assignees

@ambv ambv

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@serhiy-storchaka @gvanrossum @miss-islington @ambv @bedevere-bot @JelleZijlstra @the-knights-who-say-ni