[issue32911] Doc strings no longer stored in body of AST

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:


New changeset d6bbb57855da3f92521edd901f49705be46fb203 by Serhiy Storchaka in 
branch '3.7':
bpo-32911: Update the historical note about the magic number. (GH-7272)
https://github.com/python/cpython/commit/d6bbb57855da3f92521edd901f49705be46fb203


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:


New changeset 941ec210aaef517cf818b043ec28345962f78465 by Serhiy Storchaka in 
branch 'master':
bpo-32911: Add the historical note about the magic number. (GH-7273)
https://github.com/python/cpython/commit/941ec210aaef517cf818b043ec28345962f78465


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
pull_requests: +6900

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
pull_requests: +6899

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Nick Coghlan]

Nick Coghlan  added the comment:

In that case, closing this as resolved. Folks interested in the idea of a 
"suite" compilation mode to compile a series of statements without 
special-casing the first one as a potential docstring, as well as the idea of a 
dedicated DocString AST node may want to open new RFEs for 3.8 :)

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

I concur.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[INADA Naoki]

INADA Naoki  added the comment:

I think it's OK.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Ned Deily]

Ned Deily  added the comment:

Thank you all for your input and thank you, Inada-san and Serhiy, for the PRs.  
Is there anything more that needs to go in for 3.7.0b5 tomorrow so that this 
can be tested before the release candidate?  If not, can we now close this 
again?  If new issues arise with b5 or later, we should probably track with 
another issue.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:


New changeset 73cbe7a01a22d02dbe1ec841e8779c775cad3d08 by Serhiy Storchaka in 
branch 'master':
bpo-32911: Revert bpo-29463. (GH-7121) (GH-7197)
https://github.com/python/cpython/commit/73cbe7a01a22d02dbe1ec841e8779c775cad3d08


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
pull_requests: +6831

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:


New changeset 2641ee5040abb090e66e4ff80c33b76729b36e75 by Serhiy Storchaka in 
branch '3.7':
bpo-32911: Revert bpo-29463. (GH-7121)
https://github.com/python/cpython/commit/2641ee5040abb090e66e4ff80c33b76729b36e75


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[STINNER Victor]

Change by STINNER Victor :


--
nosy:  -vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Benjamin Ragan-Kelley]

Benjamin Ragan-Kelley  added the comment:

That should work well for us. Our patches for this are all conditional on the 
module body being empty, so reverting causes us no issues at all.

Thank you!

--
nosy: +Benjamin Ragan-Kelley

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Ned Deily]

Ned Deily  added the comment:

OK, since I believe everyone who has spoken up so far has chosen B or a 
variation on it, I think we can eliminate option A.  And there also seems to be 
a consensus so far among the core developers who have spoken up for the 
approach in PR 7121.  Before we commit to it and produce 3.7.0b5, I really 
would like to hear from at least one of the downsteamers that this approach 
seems OK for 3.7.0. mbussonn? MIN RK?

Thank you all for your (prompt) help so far with this!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[INADA Naoki]

INADA Naoki  added the comment:

I'm +1 on PR-7121 too.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Nick Coghlan]

Nick Coghlan  added the comment:

After looking at potential implementation approaches, I believe the following 
would be the ASDL code generator changes needed to present the docstring as 
part of the body in the Python API, while keeping it as a separate attribute in 
the C API:

* in ast2obj, add code in the converter postamble to inject the docstring as 
body[0]
* in obj2ast, add code in the converter preamble to pop and discard body[0] 
when docstring is not None
* in ast_type_init, add code to either inject the docstring as body[0] (if the 
docstring is set), or else to extract and set the docstring based on body[0]

Since CPython's own AST optimizer works at the C API layer, we then wouldn't 
need to revert any of the changes that relied on the separation of the 
docstring out to its own attribute.

At the Python level, some differences would still be visible though:

- there'd be a node for the docstring in the AST, but the value would still be 
optimised out of the resulting code object
- the first line number of code objects would still change to be that of the 
first non-docstring line
- the synthesised docstring node would always claim to be the line before the 
first non-docstring line (since escape characters mean we can't just count line 
feeds in the docstring)

Given the complexity of the required changes to the ASDL code generator, and 
the Python level API differences that would still remain, I think Serhiy's full 
reversion patch is going to be the more reliable option at this late stage of 
the release process.

While it isn't nice for the folks that already adapted their code to cope with 
the earlier beta releases, it's the lowest risk approach that lets us get 3.7.0 
out the door without unintended consequences, while allowing the issue to be 
revisited for 3.8 with greater awareness of the potential backwards 
compatibility and code migration issues.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Nick Coghlan]

Nick Coghlan  added the comment:

Just noting that I'm going to attempt an alternative, hopefully lower impact, 
approach at implementing the reversion, which will be to modify 
Parser/asdl_c.py to inject the following constructor postamble for ASDL nodes 
that define both a "body" and a "docstring" field:

1. If docstring is set, also insert it as the first entry in the node's body 
sequence (this is enough to get AST compilation back to behaving the way it did 
in 3.6, since the docstrings will be reinserted as `body[0]` even after the AST 
compilation process took them out)
2. If docstring is not set, check if the first entry is usable as a docstring, 
and if so, also set it as the docstring (this would restore compatibility with 
node manipulation code that expects to be able to pass a docstring as body[0])

In 3.8, we'd then decide how to eliminate the duplication of information 
between the two locations, but do so armed with the ability to emit deprecation 
warnings for code which isn't setting the node docstring attribute explicitly.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Matthias Bussonnier]

Matthias Bussonnier  added the comment:

> Just to quickly touch on Matthias' question about opt-in or deprecations, a 
> key thing to note is the ast module is automatically generated from the ASDL 
> file, so either of those approaches would require developing a new mechanism 
> in the code generator to support either approach.

Yes, I was mostly thinking of `compile(..., mode, flags=PyAstOnly)` as to where 
the deprecation could be. 

Thinking a bit more about the `compile`'s `multiline` option, that would also 
be quite useful to allow some construct that are so far forbidden, like 
top-level `await`.

I'm thinking that splitting `exec` into two options:
  - `module` which could do what current 3.7 does and find docstrings plus do 
some optimisation and sanity check if necessary, prevent top-level await. 
  - `multiline` which would treat the source as a sequence of statement, allwo 
top level `await` ... etc.

this could allow both to evolve and have their individual advantage, leaving 
`exec` unchanged for legacy reasons.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Brett Cannon]

Brett Cannon  added the comment:

Just to quickly touch on Matthias' question about opt-in or deprecations, a key 
thing to note is the ast module is automatically generated from the ASDL file, 
so either of those approaches would require developing a new mechanism in the 
code generator to support either approach.

--
nosy: +brett.cannon

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

PR 7121 is based on PR 5927 and goes one step further. It doesn't introduce a 
new AST type DocString. Third-party code should not need any adaptation for 3.7.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
keywords: +patch
pull_requests: +6756
stage: commit review -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[INADA Naoki]

INADA Naoki  added the comment:

> B. Revert the feature now for 3.7.0, retargeting for 3.8, and produce a 
> 3.7.0b5 on a somewhat shorter cycle to allow downstream users to adapt to the 
> removal.

Please note that it can't be reverted simply.
The change was introduced to ease AST-layer optimization (`"abc" + "def"` is 
not docstring, but `"abcdef"` is docstring.)
And we implemented some AST-layer optimization already.

There are two ways to revert the change:

B1. Just remove `.docstring` attribute and implement hack to distinguish string 
and docstring in some places.

B2. Remove `.docstring` attribute, but introduce `DocString` statement.  This 
was my patch (PR-5927)

In case of B2, thirdparty libraries should follow the change anyway.
In case of IPython, `DocString(s="spam")` may be replaced with 
`Expr(Str(s="spam"))`

In case of B1, I hadn't tried it yet and I don't know how difficult it is.  
I'll try but I can't say estimated time for now.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Matthias Bussonnier]

Matthias Bussonnier  added the comment:

Łukasz Langa wrote:

> Inadasan, I think what we should do is to amend `ast.parse()` and `compile()` 
> docs that mode="exec" treats given code as a module, we should even give the 
> docstring handling as an example.

That is what I proposed in https://bugs.python.org/issue33477
and in PR https://github.com/python/cpython/pull/6973

The other surprise, is that even is I  knew AST were change and had a docstring 
field I was not expecting `compile()` to be affected.


I think that the change for ast to have a docstring field is a good one from an 
API point of view for ast nodes. But It should probably be opt-in, at least 
with a deprecation period to make it the default. The other issue is that as it 
affect only sequence of statement where the first node is (was) a string, it 
can lead to really subtle difference in execution. 



MinRk said:
> The only affected case for us is interactively typed string literals in 
> single statement cells not displaying themselves as results

It might affect our downstream consumers that have syntax/ast transformation 
like sage, sympy on top of IPython. I doubt we have any but it's a possibility. 

While I would prefer (B) as well, I see how the new behavior can be useful in 
some case, and strongly also support the addition of a `multiline` mode, which 
correspond to what 3.6 and before were doing with `exec`. The other possibility 
is to leave `exec` and `ast.parse` with 3.6 behavior, and include a `'module'` 
mode that does reflect current 3.7 behavior.

--
nosy: +mbussonn

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Nick Coghlan]

Nick Coghlan  added the comment:

Ouch, I'd completely missed the fact that this would affect the parsing of bare 
strings to a simple AST (I was focused on functions and classes, as in Mark's 
original example).

So even though I'm the author of https://bugs.python.org/issue11549#msg193656 
(where I note that we consider it reasonable for AST manipulation code to 
require updates when going between major Python versions), I'm reluctantly 
voting "B", since there's a difference between "some AST manipulation code will 
need to change to account for new node types and arrangements" and "all code 
calling ast.parse with the default mode and processing the top level node will 
need to change to account for docstrings now being omitted from the module 
body, with no readily available quick fix to get the old behaviour back".

(Note that in voting for option B, I'm really only objecting to the change when 
it comes to Module AST nodes - rather than full reversion, I'd also be OK with 
a change that duplicated the new docstring attribute as body[0] for modules, 
while continuing to eliminate the redundancy for functions and classes - this 
would be a more selective variant of Mark's "Option 1" proposal from back in 
February).

--
nosy: +ncoghlan

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Eitan Adler]

Change by Eitan Adler :


--
nosy: +eitan.adler

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Min RK]

Min RK  added the comment:

In the A/B vote, I cast mine for B, for what it is worth, but it is not 
strongly held.

>From the IPython side, I don't view our particular issue as a major regression 
>for users. The only affected case for us is interactively typed string 
>literals in single statement cells not displaying themselves as results. Since 
>the same string is necessarily already displayed in the input, this isn't a 
>huge deal. This is pretty rare (maybe folks do this while investigating 
>unicode issues?) and we can handle it by recompiling empty modules with 
>'single' instead of the usual 'exec' that we use because most IPython inputs 
>are multi-statement cells coming from things like notebooks. It's relevant to 
>note that *any* logic in the cell, e.g. `"%i" % 1` or additional statements 
>have no issues.

The proposed 'muliline' or 'interactive' compile mode would suit IPython very 
well, since that's what we really want - single * N, not actually a module, and 
this is illustrated by the way we do execution: compile with exec, then iterate 
through module.body and run the nodes one at a time.

--
nosy: +minrk

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Ned Deily]

Ned Deily  added the comment:

Based on more recent discussions and indirect feedback from downstream users 
(primarily the recent IPython experience), I am reluctantly re-opening this 
issue for 3.7.0.  I take responsibility for encouraging us earlier in the beta 
phase to continue with the feature as it stood, thereby prioritizing schedule 
over any technical issues.  Although it is now very late in the 3.7.0 cycle, 
with the newly expressed concerns my opinion has changed: I think we owe it to 
our downstream users to think about this one more time.

I agree with Benjamin's remark that it is very late in the 3.7 cycle to be 
considering new or revised APIs for 3.7.0.  So it seems to me we have only two 
practical alternatives at this point:

A. Proceed with 3.7.0rc1 as planned with the docstrings feature as it now 
stands and consider API changes for 3.8.

or

B. Revert the feature now for 3.7.0, retargeting for 3.8, and produce a 3.7.0b5 
on a somewhat shorter cycle to allow downstream users to adapt to the removal.

I don't know how controversial making this decision will be but I think we need 
to move quickly as this is now *the* blocker for 3.7.0.  Coming now at the 
start of a weekend, in some countries a 3-day holiday weekend, I do want to 
make sure that the major stakeholders involved with this issue get a chance to 
"vote", although ultimately it will be up to the release manager (me) to make 
the final decision.  So, to be clear, we will not be deciding here and now what 
the API should be if we were to retarget for 3.8; please save that discussion 
for later.  The only question open right now is a vote for either option A 
(proceed as is) or option B (revert for 3.7 and retarget).  Let's set a limit 
for "voting" until Tuesday 2018-05-29 18:00 UTC (to cover the holiday weekend) 
with the proviso that if a clear consensus one way or the other appears before 
that we may cut the period short and proceed to implemention.

I know this is not a pleasant task especially at this late date.  I apologize 
to everyone, especially to Inada-san, for dragging this out.  I certainly 
appreciate the hard work that has gone into this feature so far and look 
forward to seeing it in either 3.7 or 3.8.

--
priority:  -> release blocker
resolution: wont fix -> 
stage: resolved -> commit review
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Ned Deily]

Ned Deily  added the comment:

For the record, Serhiy commented on this issue today in a thread on the 
python-committers list:

"I have doubts about two issues. I feel the responsibility for them because I 
had the opportunity to solve them before, but I lost it.

1. Changes in the AST. Few third-party projects was broken by it and already 
are fixed. I suppose yet few projects will be changed after 3.7 be released. It 
is interesting that IPython was broken in different way than other projects. It 
was needed to reintroduce the docstring in the list of statements, effectively 
reverting the 3.7 change. IPython allows to enter several statements at prompt, 
and therefore it compiles them with the 'exec' mode instead of 'single' as the 
CPython REPL and IDLE shell. Currently CPython doesn't allow you to paste 
arbitrary script like the following:

if a:
b
if c:
d

You need to add an empty line between top-level complex statements. If one time 
CPython will add support of pasting several statements without empty lines 
between, it might need to add the same hack as IPython. I afraid that we might 
be needed to change AST again, in 3.7.1 or in 3.8.0."

And later:

"Inada's patch looked complex (actually it mostly restored the code before his 
previous change). We didn't know about IPython and we decided that it is not 
worth to change this code at this stage (after beta2). And definitely it will 
be later to do this after rc1."

Full context/content starting here:
https://mail.python.org/pipermail/python-committers/2018-May/005467.html

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Benjamin Peterson]

Benjamin Peterson  added the comment:

Regardless of the value of .docstring change, it seems it had some unexpected 
consequences. I think providing a new mode for compile mulitline statements is 
a reasonable way to address those consequences going forward. That said, it's 
extremely late in the lifecycle of 3.7 to be thinking about new APIs. How about 
rolling back the whole .docstring change for 3.7? Then, it can be landed again 
with compile(..., mode='multiline) for 3.8.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[STINNER Victor]

STINNER Victor  added the comment:

IMHO Python 3.7 will be simpler to process, since it avoids to check if the 
first node is a docstring or not.

I agree that it's a backward incompatible, but it has been done on purpose. In 
fact, there is no warranty about backward compatibility on the genreated AST 
between Python versions (like 3.6 vs 3.7).

As Lukasz wrote, AST of Python 3.6 already omit a lot of information about 
formatting, we are far from a 1:1 mapping between code and AST. For example, 
tools to "unparse" AST breaks the formatting. The Python 'ast' module is just 
not appropriate for such use case. There are other solutions for that, like 
lib2to3.

Having a docstring attribute *and* a docstring node seem to be redundant to be, 
or more like a bug. Redundant data can quickly lead to inconsistencies.

The current trend is more to move the AST away from the original Python source 
code, to produce better AST, especially in term of performance. That's also why 
I added the ast.Constant node type ;-) And why many peephole optimizations have 
been reimplemented at the AST level, rather than on the bytecode level.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Ned Deily]

Ned Deily  added the comment:

> * Merge my patch in 3.7rc1, or

Sorry but we are not going to revisit this decision for 3.7.0.  It's too late!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[INADA Naoki]

INADA Naoki  added the comment:

If Python 3.7 is released with current AST form, I don't want to change it 
again.

I prefer

* Merge my patch in 3.7rc1, or
* Add compile(..., "multi") mode for "sequence of statements"

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Maybe we will need to return to this issue in future and merge Inada's patch. 
IPython has been needed to add a workaround for this change. [1] And maybe 
Python will be needed to add that workaround if once it will add the support of 
pasting multiple complex statements without empty lines between.

[1] https://github.com/ipython/ipython/issues/11133

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Łukasz Langa]

Łukasz Langa  added the comment:

Oh, and Francis, you might actually want to use lib2to3.pytree instead, which 
will provide you with a Concrete Syntax Tree ("lossless AST").  That tree will 
keep information like whitespace, comments, and other semantically 
insignificant tokens like commas and parentheses.

This tree will never modify your input whereas in the AST we might be doing 
more modifications in the future (like cutting `if` branches that we statically 
know are always False, and so on).

Granted, the lib2to3 tree is a bit awkward to query since the concrete tree 
exposes the raw Python grammar and not the simplified AST grammar.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Łukasz Langa]

Łukasz Langa  added the comment:

Inadasan, what Francis is referring to is that people often use `ast.parse()` 
to parse code fragments, not entire modules.

For example:

  >>> some_code = '"just a string"'
  >>> some_code_ast = ast.parse(some_code)

The way `ast.parse()` works, it always considers the input to be a module:

  >>> some_code_ast
  <_ast.Module object at 0x109537a18>

Before Python 3.7, you could parse out a string from such "module":

  >>> some_code.body
  [<_ast.Expr object at 0x1079aaa20>]
  >>> some_code.body[0].value
  <_ast.Str object at 0x107a45198>

However, starting with Python 3.7, this module is empty:

  >>> some_code_ast.body
  []
  >>> some_code_ast.docstring
  'just a string'

`ast.literal_eval()` does not have this problem.


And now: where is the bug?  I think the user code has a bug trying to parse a 
code fragment with `mode="exec"`.  Instead of using the default `mode="exec"`, 
it seems to me `mode="single"` fits the bill better.  In some cases the 
programmer might prefer `mode="eval"` if they don't want to allow statements at 
all.

Inadasan, I think what we should do is to amend `ast.parse()` and `compile()` 
docs that mode="exec" treats given code as a module, we should even give the 
docstring handling as an example.

Francis, if your use case requires passing multiple statements/expressions at 
once and a string in the first statement has semantic meaning, please provide 
us with a concrete example.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[INADA Naoki]

INADA Naoki  added the comment:

> in practice `ast.parse()` is often used with smaller fragments of source code 
> that don't directly correspond to a complete module, and this behaviour makes 
> no sense in such cases.
>
> The resulting bug in kdev-python took a while to track down (because I had no 
> immediate reason to suspect this change), and will be somewhat awkward to 
> workaround.

Would you elaborate it more, please?
Without concrete example, we cannot to decide how it's important problem.

> I would prefer that this be reverted; it's likely to break a variety of users 
> in strange ways.

I don't want to revert, see discussions in this issue and PR-5927.

And would you give us real world example of "variety of users"?
How many applications use ast.parse for such way?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Francis Herne]

Francis Herne  added the comment:

Note:

Because this also applies to module-level docstrings, a rather strange effect 
that, for example, `ast.parse("'foo'")` is now an empty module.

While the root node is always an instance of `ast.Module`, in practice 
`ast.parse()` is often used with smaller fragments of source code that don't 
directly correspond to a complete module, and this behaviour makes no sense in 
such cases.

The resulting bug in kdev-python took a while to track down (because I had no 
immediate reason to suspect this change), and will be somewhat awkward to 
workaround.

I would prefer that this be reverted; it's likely to break a variety of users 
in strange ways.

--
nosy: +flherne

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Ned Deily]

Ned Deily  added the comment:

Thanks everyone for your inputs.  Let's go with the status quo -> closing this 
issue.

--
priority: deferred blocker -> 
resolution:  -> wont fix
stage:  -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Mark Shannon]

Mark Shannon  added the comment:

I still think that option 2 would be best, but given that it is late in the 
release cycle, I will accept the status quo.

A couple of comments for the record:

Constant folding occurs after AST creation, so doesn't make the AST worse.

The parse tree created by lib2to3 is a concrete parse tree, not an AST. 
However, it might would sense (as Łukasz suggests) to use it as a basis for an 
AST designed for tooling and leave the AST generated by the C parser for 
bytecode generation.


Happy to close this issue now, unless anyone else has something to add.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Łukasz Langa]

Łukasz Langa  added the comment:

Python's AST already doesn't include a lot of syntax that has no runtime effect:
- comments;
- whitespace and punctuation.

The situation is now made "worse" with constant folding.  You won't have full 
line information on the full expression, you don't even fully *see* expressions 
as syntactically provided by user code.  The docstring is just a special case 
of that.

As a counter example, it still includes function-level type annotations which 
are not evaluated. Clearly we don't have an explicit rule about this.

If you asked *me*, I would say Python provides a "lossy" AST and this is the 
one it itself uses.  It also provides a "lossless" AST and this one is in 
lib2to3.  It's unfortunate that the latter is not documented and kind of 
neglected.  This is something I want to address soonish.

Summing up, I'm in favor of option 3.  The AST preserves the docstring, it 
simply doesn't have information on where exactly it lives.  In exchange, the 
tree makes it easier to *find* the docstring and easier to find the first 
instruction (as Serhiy points out, the first statement is now always code to be 
executed).

Quoting literally the second sentence of the `ast` documentation:
> The abstract syntax itself might change with each Python release.

I am not against Option 2 since it's already implemented but I feel it's a 
pretty big change, pretty late in the release process for 3.7.  The amount of 
work required *just* to preserve the line number of the docstring (which can be 
reasonably guessed anyway) seems silly to me.

--
nosy: +lukasz.langa

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[INADA Naoki]

INADA Naoki  added the comment:

There are no enough +1 for merging GH-5927 yet on the ML discussion.
https://mail.python.org/pipermail/python-dev/2018-February/152311.html

@Mark.Shannon, would you comment your opinion to the ML thread?

For now, I don't want to break tools supporting Python 3.7 a1~b2 already.
Although I will select (2) if there is time machine...

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Ned Deily]

Ned Deily  added the comment:

Since we are already past the 3.7.0b2 cutoff time and there does not seen to be 
a consensus that the current 3.7 behavior needs to change and the proposed 
change is quite large, I think we should not change anything now for b2.  You 
can have a discussion on python-dev and, if there is a consensus for change, we 
can look at it for b3.

--
priority: release blocker -> deferred blocker

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Sorry, Python-Dev, of course.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[INADA Naoki]

INADA Naoki  added the comment:

Python-list? -dev? -ideas?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Would be nice to discuss this design question on Python-list.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[INADA Naoki]

INADA Naoki  added the comment:

I'm implementing (2).  Please check GH-5927.

--
keywords:  -patch
stage: patch review -> 

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[INADA Naoki]

Change by INADA Naoki :


--
keywords: +patch
pull_requests: +5698
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[INADA Naoki]

INADA Naoki  added the comment:

I tried this patch:

diff --git a/Python/ast.c b/Python/ast.c
index e2092f0f85..93be2bc839 100644
--- a/Python/ast.c
+++ b/Python/ast.c
@@ -3537,9 +3537,9 @@ docstring_from_stmts(asdl_seq *stmts)
 if (s->kind == Expr_kind && s->v.Expr.value->kind == Str_kind) {
 string doc = s->v.Expr.value->v.Str.s;
 /* not very efficient, but simple */
-memmove(_seq_GET(stmts, 0), _seq_GET(stmts, 1),
-(stmts->size - 1) * sizeof(void*));
-stmts->size--;
+//memmove(_seq_GET(stmts, 0), _seq_GET(stmts, 1),
+//(stmts->size - 1) * sizeof(void*));
+//stmts->size--;
 return doc;
 }
 }

But I got "SyntaxError: from __future__ imports must occur at the beginning of 
the file".

docstring is very special while it looks like just a single string literal 
statement...

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[Ned Deily]

Ned Deily  added the comment:

Is this going to get resolved in time for 3.7.0b2?  If we need to change this 
behavior, I really do not want to delay this for b3 which is when the 3.7.0 ABI 
freezes; otherwise such a major behavior change would likely need to wait until 
3.8.

--
priority: normal -> release blocker

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32911] Doc strings no longer stored in body of AST

[ppperry]

Change by ppperry :


--
title: Doc strings omitted from AST -> Doc strings no longer stored in body of 
AST

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com