[proxy] github.com — ← back | site home | direct (HTTPS) ↗ | proxy home | ◑ dark◐ light
/ cpython Public

Conversation

Copy link
Member

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

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

https://bugs.python.org/issue33475

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 added type-bug An unexpected behavior, bug, or error needs backport to 3.7 labels May 12, 2018
serhiy-storchaka requested a review from ambv May 12, 2018 22:31
Copy link
Member

gvanrossum commented May 13, 2018

@ambv should review it

JelleZijlstra reviewed May 13, 2018
View reviewed changes
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
Copy link
Member

JelleZijlstra May 13, 2018

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 merged commit 64fddc4 into python:master May 17, 2018
Copy link
Contributor

miss-islington commented May 17, 2018

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

Copy link
Contributor

ambv commented May 17, 2018

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.

Copy link
Contributor

miss-islington commented May 17, 2018

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

Copy link
Contributor

ambv commented May 17, 2018

Oops, looks like 3.7 needs help.

serhiy-storchaka deleted the ast-unparse-omit-parens branch May 17, 2018 06:25
Copy link
Member Author

serhiy-storchaka commented May 17, 2018

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
[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>
Copy link

bedevere-bot commented May 17, 2018

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
[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 left review comments

ambv Awaiting requested review from ambv

Assignees

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