◐ Shell
reader mode source ↗
Skip to content

bpo-32911: Remove the docstring attribute of AST types#7121

Merged
serhiy-storchaka merged 4 commits into
python:3.7from
serhiy-storchaka:remove-ast-docstring-attr-3.7
May 29, 2018
Merged

bpo-32911: Remove the docstring attribute of AST types#7121
serhiy-storchaka merged 4 commits into
python:3.7from
serhiy-storchaka:remove-ast-docstring-attr-3.7

Conversation

@serhiy-storchaka

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

Copy link
Copy Markdown
Member

It is based on #5927 and goes one step further to Python 3.6. It doesn't introduce a new AST type DocString.

https://bugs.python.org/issue32911

@ned-deily ned-deily assigned ncoghlan and benjaminp and unassigned benjaminp and ncoghlan May 25, 2018
@ned-deily ned-deily requested review from benjaminp and ncoghlan May 25, 2018 18:37
ncoghlan
ncoghlan requested changes May 27, 2018
View reviewed changes

@ncoghlan ncoghlan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hide comment

Thanks for this Serhiy. In addition to the b5 NEWS entry, the only other item I see missing here is a magic number bump for the bytecode (so any debug and opt-level 1 bytecode generated with the earlier alphas and betas gets regenerated with docstrings included again).

@bedevere-bot

bedevere-bot commented May 27, 2018

Copy link
Copy Markdown

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@ncoghlan

ncoghlan commented May 27, 2018

Copy link
Copy Markdown
Contributor

Note that while the previous 3.7.0a1 NEWS entry is modified by the current patch, there'll need to be a dedicated NEWS entry for the reversion that will then appear under 3.7.0b5.

@serhiy-storchaka

serhiy-storchaka commented May 28, 2018

Copy link
Copy Markdown
Member Author

Sorry, I don't know Git well still, and loss a news entry when applied Inada's patch.

I didn't think that a magic number bumping is needed, because this PR affects AST. But it also affects the first line number of nodes with docstrings. I'll bump a magic number.

ned-deily
ned-deily requested changes May 28, 2018
View reviewed changes

@ned-deily ned-deily left a comment

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.

Hide comment

A couple of wording improvements

@bedevere-bot

bedevere-bot commented May 28, 2018

Copy link
Copy Markdown

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@serhiy-storchaka

serhiy-storchaka commented May 29, 2018

Copy link
Copy Markdown
Member Author

I have made the requested changes; please review again.

@bedevere-bot

bedevere-bot commented May 29, 2018

Copy link
Copy Markdown

Thanks for making the requested changes!

@methane, @ned-deily, @ncoghlan: please review the changes made to this pull request.

@serhiy-storchaka serhiy-storchaka merged commit 2641ee5 into python:3.7 May 29, 2018
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request May 29, 2018
@serhiy-storchaka @methane
bpo-32911: Revert bpo-29463. (pythonGH-7121) …
728e537 
Remove the docstring attribute of AST types and restore docstring
expression as a first stmt in their body.

Co-authored-by: INADA Naoki <methane@users.noreply.github.com>
@serhiy-storchaka serhiy-storchaka deleted the remove-ast-docstring-attr-3.7 branch May 29, 2018 09:06
serhiy-storchaka added a commit that referenced this pull request May 29, 2018
@serhiy-storchaka @methane
bpo-32911: Revert bpo-29463. (GH-7121) (GH-7197) …
73cbe7a 
Remove the docstring attribute of AST types and restore docstring
expression as a first stmt in their body.

Co-authored-by: INADA Naoki <methane@users.noreply.github.com>
@ncoghlan

ncoghlan commented May 29, 2018

Copy link
Copy Markdown
Contributor

Thanks @serhiy-storchaka!

@berkerpeksag berkerpeksag mentioned this pull request May 29, 2018
Closed
@Kodiologist Kodiologist mentioned this pull request May 31, 2018
Closed
Kodiologist added a commit to Kodiologist/hy that referenced this pull request Jun 6, 2018
@Kodiologist
Update docstring handling for Python 3.7 …
16ec46a 
See python/cpython#7121 .
@carlwgeorge carlwgeorge mentioned this pull request Jun 15, 2018
Merged
carlwgeorge added a commit to carlwgeorge/jedi that referenced this pull request Jun 15, 2018
@carlwgeorge
Parse correct AST attribute for version …
d2640d3 
Earlier development versions of Python 3.7 added the docstring field to
AST nodes.  This was later reverted in Python 3.7.0b5.

https://bugs.python.org/issue29463
python/cpython#7121
@carlwgeorge carlwgeorge mentioned this pull request Jun 15, 2018
Merged
davidhalter pushed a commit to davidhalter/jedi that referenced this pull request Jun 16, 2018
@carlwgeorge @davidhalter
Parse correct AST attribute for version …
ff4a773 
Earlier development versions of Python 3.7 added the docstring field to
AST nodes.  This was later reverted in Python 3.7.0b5.

https://bugs.python.org/issue29463
python/cpython#7121
@isidentical isidentical mentioned this pull request Nov 11, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ncoghlan ncoghlan ncoghlan requested changes

@methane methane methane approved these changes

@ned-deily ned-deily ned-deily approved these changes

@benjaminp benjaminp Awaiting requested review from benjaminp

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@serhiy-storchaka @bedevere-bot @ncoghlan @methane @ned-deily @benjaminp @the-knights-who-say-ni