classification
Title: PEP 380 reference implementation for 3.3
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ncoghlan Nosy List: Yury.Selivanov, alex, aronacher, asvetlov, eric.araujo, gcewing, ncoghlan, rfk, rhettinger, rndblnch, ron_adam, zbysz
Priority: normal Keywords: patch

Created on 2011-03-26 03:36 by ncoghlan, last changed 2012-01-27 11:17 by ncoghlan. This issue is now closed.

Files
File name Uploaded Description Edit
f8349cbc1b26.diff ncoghlan, 2012-01-12 14:28 review
Repositories containing patches
https://bitbucket.org/ncoghlan/cpython_sandbox#pep380
http://hg.python.org/sandbox/ncoghlan#pep380
Messages (35)
msg132213 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-03-26 03:36
Creating an issue to track efforts to forward port the PEP 380 reference implementation to Python 3.3.
msg134550 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-04-27 10:26
(“Create patch” does not work for some reason, I’ll report that to the metatracker in a few days if nobody does it first)
msg134556 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-04-27 12:26
It's because it isn't a true Mercurial clone-and-update - it's a patch queue, which the review generation code doesn't know how to interpret.
msg134925 - (view) Author: Renaud Blanch (rndblnch) Date: 2011-05-01 20:24
As nick said, the repo only host a patch queue.
the patch itself is visible here:
https://bitbucket.org/rndblnch/cpython-pep380/qseries?apply=t&qs_apply=pep380

Or it can be download here in raw text:
https://bitbucket.org/rndblnch/cpython-pep380/raw/tip/pep380

I have documented how to apply the patch to a cpython clone here:
https://bitbucket.org/rndblnch/cpython-pep380/wiki
msg139145 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-06-26 04:22
OK, this isn't going to be as simple as I thought it was. The big problem is that the test suite from [1] needs to be refactored into a format that is suitable for use in regrtest. That means porting them to either unittest or doctest, as "golden output" tests of the style that Greg used are no longer accepted as part of the regression test suite.

[1] http://www.cosc.canterbury.ac.nz/greg.ewing/python/yield-from/yield_from.html
msg139471 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-06-30 11:25
Renaud has updated the patch on bitbucket to incorporate Greg's tests (still in golden output form, but in a unittest/regrtest compatible way). That's a good enough starting point for me - they can be refactored into proper assert based tests later.

The only tweak needed is to explicitly save and restore stdout and stderr, since restoring the OS level ones often does the wrong thing when running the regression test suite.
msg139529 - (view) Author: Renaud Blanch (rndblnch) Date: 2011-06-30 22:12
I've just updated the pep380-test patch to make it slightly simpler (still in golden output form though)

https://bitbucket.org/rndblnch/cpython-pep380/src/317eadf5e3e8/pep380-tests
msg140072 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-07-09 15:31
Once again got close to committing this, but then realised it is missing all the necessary documentation updates for a core language change.

I have uploaded a patch that includes all the changes from Bitbucket as well as the subsequent fixes to avoid modifying sys.stdout when the tests are run and to comply with the whitespace rules set up in the source control hooks (the ACKS, NEWS and a placeholder in whatsnew are also included).

What's missing are updates to at least:

http://docs.python.org/dev/tutorial/classes.html#generators
(a simple example showing delegation should suffice there)

http://docs.python.org/dev/reference/simple_stmts.html#the-yield-statement
http://docs.python.org/dev/reference/expressions.html#grammar-token-yield_expression
http://docs.python.org/dev/reference/simple_stmts.html#the-return-statement
(the language reference must be updated for a post PEP 380 world)

There are likely other places that should also be updated, but these are the critical ones needed before the patch can be included.
msg140110 - (view) Author: Renaud Blanch (rndblnch) Date: 2011-07-11 11:51
I can not comment on http://bugs.python.org/review/11682/, so a quick comment here: Doc/whatsnew/3.3.rst patch gives me credit together with Greg Ewing for the implementation, but I've only upgraded his patches to 3.3.
So, the following line:
+(Implementation by Greg Ewing and Renauld Blanch)
should really be:
+(Implementation by Greg Ewing)

And, as I'm on it, the proper spelling for my surname is Renaud (not Renauld as it also appears in the commit message :)
msg140115 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-07-11 12:52
I moved my personal sandbox from hg.python.org to bitbucket, so it should be easier for folks to build off the work in progress. (And fixed the typo in Renaud's name - I at least had it right in ACKS. I also reworded the draft attribution text in What's New)

I skimmed Benjamin's comments and basically agree with them. I've marked issue 11816 as a dependency as some of the new tests will be easier to refactor once dis.get_opinfo() is available.
msg143210 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-08-30 12:26
The pep380 branch in my bitbucket repo has been updated with the refactored tests that Ryan Kelly put together at the PyconAU sprints (as well as being brought up to speed with 3.x tip).

The update depends on the new dis.get_opinfo() API added by issue #11816 (which is currently with Raymond for review).

The AST validator still needs to be updated to take into account the new syntax and the patch attribution needs to be updated to account for Ryan's contribution, but this is getting fairly close to being ready.
msg143492 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-09-04 11:12
Uploaded a patch that should be complete. Note that my pep380 branch is based on my get_opinfo branch (see #11816), so if you're applying patches manually rather than updating directly from my sandbox with hg, you'll need to apply the latest patch from #11816 before applying this one.

Raymond: assigning to you, since you said you wanted to review the dis.get_opinfo changes before they went in, and those changes are a dependency for the updated PEP 380 test suite.
msg143493 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-09-04 11:18
Actually, not assigning to Raymond for review yet, after all - I just noticed there are some of Benjamin's review comments relating to cosmetic details rather than functionality that I still need to address.

I'll kick it in Raymond's direction once I've dealt with those (probably next weekend, but possibly some time this week)
msg143510 - (view) Author: Zbyszek Jędrzejewski-Szmek (zbysz) * Date: 2011-09-05 09:19
I've created some documentation... The patches are the bitbucket repo.

Nothing is added to the tutorial, because I think that this isn't material for a newcomer to python. The tutorial doesn't mention generator.throw() and send() either, just talks a little about writing simple generator functions.
msg144325 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-09-20 12:29
I have updated the bitbucket repo with changes to address most of Benjamin's review comments.

A few points of note:
- I agree in principle with the idea of splitting Yield and YieldFrom into distinct AST nodes, but I'd prefer to focus on getting the current implementation into the main repo first
- in cleaning up the handling of missing send/close/throw attributes on subiterators I discovered that the PyObject_CallMethod* APIs were discarding exception information and coercing everything to a terse AttributeError. The branch now changes them to allow the error reported by the underlying call to PyObject_GetAttr to pass through unmodified.
- the generator close() method treats an AttributeError as expected when looking for a close() method on the subiterator, but uses WriteUnraisable to deal with anything else.
- I share Benjamin's suspicion that some of the INCREF/DECREF pairs in genobject.c aren't actually necessary, but I don't think it's worth messing with them at this stage.

I haven't looked at Zbyszek's proposed doc changes at this point - the pull request has a lot of irrelevant line wrapping changes in it, so it makes it hard to review.
msg144350 - (view) Author: Zbyszek Jędrzejewski-Szmek (zbysz) * Date: 2011-09-20 19:42
Here are the un-reflowed documentation changes, as a patch this time. I've split the changes in two:
0001 is the real documentation change
0002 is the link fixes [optional].
msg144378 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-09-21 14:26
Doc patch 002 is not strictly related to PEP 380 but pertains more to #10289.
msg145144 - (view) Author: Armin Ronacher (aronacher) * (Python committer) Date: 2011-10-07 20:50
A little bit of input on this issue.  Considering that exceptions are now getting keyword arguments for things like import errors and other things for attributes I would find it much better if StopIteration would follow that as well (StopIteration(value=42) instead of StopIteration(42)).
msg146695 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-10-31 12:58
Bitbucket repo and attached patch updated relative to latest get_opinfo branch (which in turn was updated to apply cleanly against current CPython tip).

(I still need to incorporate the doc updates and look into adding keyword argument support)
msg147320 - (view) Author: Yury Selivanov (Yury.Selivanov) * Date: 2011-11-08 22:56
It looks like there is a memory leak bug (on StopIteration exception instances).

Attached is the test to expose it.

It seems that adding 'Py_DECREF(e);' after 'PyErr_SetObject(PyExc_StopIteration, e);' in 'genobject.c' fixes the leak.
msg147347 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-11-09 10:17
In reviewing Zbyszek's doc updates and comparing them against the Grammar, I discovered a gratuitous change in the implementation: it allows a bare (i.e. no parentheses) 'yield from' as an argument to a function, even when there's multiple arguments. By contrast, an ordinary yield expression requires parentheses everywhere other than when it's being used as a statement, or as the sole expression on the RHS of an assignment.

I'll add a new test to ensure "yield from x" requires parentheses whenever "yield x" requires them (and fix the Grammar file on the implementation branch accordingly).

I've also pushed a fix to the branch for the refleak Yury pointed out (once I actually ran regrtest in refleak hunting mode it also pointed out that there was a problem, so I know his suggested change definitely fixed it).
msg147348 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-11-09 11:00
OK, the bitbucket repo now has a more sane version of the new Grammar ('yield from' now requires parentheses wherever 'yield' does).

The updated test_grammar does a more thorough check of the expected acceptance and rejection of yield expressions in various situations and the overly sensitive test in test_parser (that checked for an exact expected parse tree output) is gone.
msg147404 - (view) Author: Zbyszek Jędrzejewski-Szmek (zbysz) * Date: 2011-11-10 13:29
Nick Coghlan wrote:
> I don't want to completely rearrange the yield related sections of the
> language reference as part of incorporating this PEP. If you're happy
> to submit a new pull request with a minimalist change just documenting
> the new features, that would be great, otherwise I'll eventually
> figure out my own set of updates.

OK, I should have it ready before Monday.

> The cross-linking fixes are independent of this PEP, and should be
> handled as a separate tracker issue rather than being rolled into the
> PEP update.
Will do so.

Zbyszek
msg148153 - (view) Author: Zbyszek Jędrzejewski-Szmek (zbysz) * Date: 2011-11-22 23:36
Updated doc patch 0001-2.diff: following ncoghlan's request, the bulk of yield documentation is kept in expressions.rst, and simple_stmts.rst mostly refers to the other one. (In previous version it was the other way around).

After doing this change, I still think that it is better to have most of the documentation in the _statement_ part, and keep the _expression_ part relatively simple. The reason is that yield does more than just return a value, but also throws exceptions, suspends execution, etc. This is detached from the fact that is can be used in an expression.

Unfortunately yield as an expression is the only case where a statement can be used in an expression. Therefore there aren't any other cases which could be used as a guide.
msg148163 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-11-23 00:55
*Any* expression can be used as a standalone statement and (since PEP 352 was implemented) that now applies to 'yield' as well.

PEP 352 fundamentally changed the way yield was conceptualised within the language - thinking of it as "a statement that can be used as an expression" is just plain *wrong*. It's now just an expression that happens to have a couple of special cases in the grammar to allow the parentheses to be skipped when they're distracting rather than helpful.

That is, the only reason the yield statement still exists as a separate entity is to allow us to drop the otherwise mandatory parentheses:

  (yield val)    # yield expression as statement
  yield val      # yield statement
msg148950 - (view) Author: Ron Adam (ron_adam) * Date: 2011-12-07 05:46
There is a test for 'yield from' as a function argument without the extra parentheses.

      f(yield from x)

You do need them in the case of a regular yield.

      f((yield))   or f((yield value))

Shouldn't the same rule apply in both cases?

* I'm trying to do a version of the patch with 'yield_from' as a separate item from 'yield' in the grammar, but it insists on the parentheses due to it being a yield_expr component.
msg148959 - (view) Author: Zbyszek Jędrzejewski-Szmek (zbysz) * Date: 2011-12-07 10:11
>>> f((yield))

This requirement seems unnecessary. And surprising, because f(<generator-expression>) or f('a' if 'a' else 'b') doesn't require parenthes. There's no room for confusion if parentheses were omitted in the single-argument case. (Following the rule given in documentation for generator expressions: "The parentheses can be omitted on calls with only one argument.").
msg148963 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-12-07 12:38
OK, I removed all the old files and repo links (they're still available in the history below if needed). (well, the link to Renaud's hg patch queue is gone, but that's also present in one of the early comments). (Ron: your comment makes me think you were looking at an old version of the PEP 380 updates. The most up to date published version is the tip of my BitBucket branch)

Also, don't spend a lot of time messing with functional aspects of the patch folks, especially the Grammar. 'yield from' is going to be a variant of the existing yield expressions to avoid parser ambiguity, and it's going to inherit all of the associated restrictions, including the requirement for parentheses when it's the sole argument to a function.

While that restriction isn't *necessary*, it's also irrelevant to this PEP (hence why I reverted Greg's changes that allowed it for 'yield from', but not for other yield expressions).
msg148964 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-12-07 12:42
As far as *why* yield expressions aren't special cased the way generator expressions are - just an oversight when yield expressions were added, no real grand master plan. It's not a big deal in practice, so nobody has ever cared enough to argue for fixing it.

Conditional expressions simply don't include mandatory parentheses as part of the syntax in the first place.
msg148977 - (view) Author: Ron Adam (ron_adam) * Date: 2011-12-07 16:49
Thanks for the updated links Nick.

There is a comment in the docs that recommends putting parentheses around any yield expression that returns a value.  So it is in agreement with that in the function argument case.

The grammar I used does keep it as a variant.

    yield_expr 'yield' [testlist | yield_from]
    yield_from 'from' test

The purpose of doing that is so I can do ...

    yield_expr 'yield' [testlist | yield_from | yield_raise]
    yield_from 'from' test
    yield_raise 'raise' [test ['from' test]]

The yield_raise part is only an interesting exercise to help me understand cpythons internals better.  I'm getting there and hope to be able to contribute to more bug fixes, and patches. :-)
msg151125 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-01-12 14:33
Nice milestone this evening: by incorporating doc updates based on Zbysek's efforts and dropping the explicit bytecode generation tests, I now have something that appears ready to commit *without* a dependency on the proposed dis module updates.

So, assuming nobody spots any major problems between now and then, the PEP 380 implementation should be landing in the main repo some time this weekend :)
msg151144 - (view) Author: Zbyszek Jędrzejewski-Szmek (zbysz) * Date: 2012-01-12 18:10
Some minor comments in http://bugs.python.org/review/11682/show.
msg151171 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-01-13 12:01
Committed for 3.3: http://hg.python.org/cpython/rev/d64ac9ab4cd0

Thanks to Greg for the initial effort on the PEP and reference implementation and to all involved in updating the original patch for 3.3 and getting the tests and documentation to an acceptable state.

Any issues discovered after this can be given a new tracker entry :)
msg151177 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-01-13 15:56
Kudos!
msg152085 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-01-27 11:17
Just noting that the pep380 integration branch is also available in the hg.python.org clone of my sandbox repo.
History
Date User Action Args
2012-01-27 11:17:38ncoghlansethgrepos: + hgrepo107
messages: + msg152085
2012-01-13 15:56:30eric.araujosetmessages: + msg151177
2012-01-13 12:01:19ncoghlansetstatus: open -> closed
type: enhancement
messages: + msg151171

components: + Interpreter Core
resolution: fixed
stage: resolved
2012-01-12 18:10:42zbyszsetmessages: + msg151144
2012-01-12 18:07:06zbyszsethgrepos: - hgrepo66
2012-01-12 14:33:11ncoghlansetdependencies: - Refactor the dis module to provide better building blocks for bytecode analysis
messages: + msg151125
2012-01-12 14:28:04ncoghlansetfiles: + f8349cbc1b26.diff
2012-01-12 14:27:11ncoghlansetfiles: - 494c976c41c4.diff
2012-01-12 14:27:02ncoghlansetfiles: - 0d1d76f68750.diff
2012-01-12 14:21:17ncoghlansetfiles: + 494c976c41c4.diff
2011-12-07 16:49:56ron_adamsetmessages: + msg148977
2011-12-07 12:42:31ncoghlansetmessages: + msg148964
2011-12-07 12:38:56ncoghlansetmessages: + msg148963
2011-12-07 12:31:39ncoghlansethgrepos: - hgrepo11
2011-12-07 12:31:37ncoghlansetfiles: - 0001-2.diff
2011-12-07 12:31:35ncoghlansetfiles: - test.py
2011-12-07 12:31:32ncoghlansetfiles: - issue11682_pep380_branch_20111131.diff
2011-12-07 12:31:05ncoghlansetfiles: - 0001.diff
2011-12-07 10:11:40zbyszsetmessages: + msg148959
2011-12-07 05:46:10ron_adamsetnosy: + ron_adam
messages: + msg148950
2011-11-24 22:29:14ncoghlansetfiles: + 0d1d76f68750.diff
2011-11-23 00:55:39ncoghlansetmessages: + msg148163
2011-11-22 23:36:11zbyszsetfiles: + 0001-2.diff

messages: + msg148153
2011-11-10 13:29:58zbyszsetfiles: - 0002.diff
2011-11-10 13:29:28zbyszsetmessages: + msg147404
2011-11-09 11:00:22ncoghlansetmessages: + msg147348
2011-11-09 10:17:03ncoghlansetmessages: + msg147347
2011-11-08 22:56:43Yury.Selivanovsetfiles: + test.py

messages: + msg147320
2011-10-31 12:59:20ncoghlansetfiles: - issue11682_pep380_branch_20110904.diff
2011-10-31 12:59:17ncoghlansetfiles: - pep380-missing-docs.diff
2011-10-31 12:58:57ncoghlansetfiles: + issue11682_pep380_branch_20111131.diff

messages: + msg146695
2011-10-07 20:50:21aronachersetnosy: + aronacher
messages: + msg145144
2011-09-21 14:26:55eric.araujosetmessages: + msg144378
2011-09-20 22:05:53zbyszsetfiles: + 0002.diff
2011-09-20 22:05:36zbyszsetfiles: + 0001.diff
2011-09-20 22:03:55zbyszsetfiles: - 0002-Fix-references-to-__next__-__iter__-and-nearby-refer.patch
2011-09-20 22:03:46zbyszsetfiles: - 0001-Document-the-yield-from-syntax-and-StopIteration-ret.patch
2011-09-20 19:50:47zbyszsetfiles: + 0002-Fix-references-to-__next__-__iter__-and-nearby-refer.patch
2011-09-20 19:50:10zbyszsetfiles: + 0001-Document-the-yield-from-syntax-and-StopIteration-ret.patch
2011-09-20 19:49:33zbyszsetfiles: - 0002-Fix-references-to-__next__-__iter__-and-nearby-refer.patch
2011-09-20 19:49:23zbyszsetfiles: - 0001-Document-the-yield-from-syntax-and-StopIteration-ret.patch
2011-09-20 19:43:09zbyszsetfiles: + 0002-Fix-references-to-__next__-__iter__-and-nearby-refer.patch
2011-09-20 19:42:35zbyszsetfiles: + 0001-Document-the-yield-from-syntax-and-StopIteration-ret.patch

messages: + msg144350
2011-09-20 12:29:30ncoghlansetmessages: + msg144325
2011-09-20 10:27:18ncoghlansetfiles: + pep380-missing-docs.diff
2011-09-20 08:40:04ncoghlansetfiles: - pep380-missing-docs.diff
2011-09-05 09:19:22zbyszsetnosy: + zbysz

messages: + msg143510
hgrepos: + hgrepo66
2011-09-04 11:18:57ncoghlansetmessages: + msg143493
2011-09-04 11:12:49ncoghlansetfiles: + issue11682_pep380_branch_20110904.diff

messages: + msg143492
2011-08-30 12:26:17ncoghlansetnosy: + rhettinger
messages: + msg143210
2011-08-22 11:36:26rfksetnosy: + rfk
2011-07-11 12:53:00ncoghlansethgrepos: + hgrepo40
dependencies: + Refactor the dis module to provide better building blocks for bytecode analysis
messages: + msg140115
2011-07-11 11:51:18rndblnchsetmessages: + msg140110
2011-07-09 15:31:37ncoghlansetfiles: + pep380-missing-docs.diff

nosy: + gcewing
messages: + msg140072

keywords: + patch
2011-06-30 22:12:17rndblnchsetmessages: + msg139529
2011-06-30 11:25:40ncoghlansetmessages: + msg139471
2011-06-29 06:21:51ncoghlanlinkissue11549 dependencies
2011-06-26 10:44:54asvetlovsetnosy: + asvetlov
2011-06-26 04:22:52ncoghlansetmessages: + msg139145
2011-06-26 03:14:15ncoghlansetassignee: ncoghlan
2011-05-01 20:24:22rndblnchsetmessages: + msg134925
2011-04-27 12:26:14ncoghlansetmessages: + msg134556
2011-04-27 10:26:04eric.araujosetnosy: + eric.araujo
messages: + msg134550
2011-04-26 06:32:14Yury.Selivanovsetnosy: + Yury.Selivanov
2011-03-30 20:05:26rndblnchsetnosy: + rndblnch
2011-03-26 03:42:30alexsetnosy: + alex
2011-03-26 03:36:45ncoghlancreate