Repository navigation
PEP 380 reference implementation for 3.3 #55891
Description
| BPO | 11682 |
|---|---|
| Nosy | @rhettinger, @ncoghlan, @mitsuhiko, @merwok, @alex, @asvetlov |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = 'https://github.com/ncoghlan'
closed_at = <Date 2012-01-13.12:01:19.860>
created_at = <Date 2011-03-26.03:36:45.133>
labels = ['interpreter-core', 'type-feature']
title = 'PEP 380 reference implementation for 3.3'
updated_at = <Date 2012-01-27.11:17:38.832>
user = 'https://github.com/ncoghlan'bugs.python.org fields:
activity = <Date 2012-01-27.11:17:38.832>
actor = 'ncoghlan'
assignee = 'ncoghlan'
closed = True
closed_date = <Date 2012-01-13.12:01:19.860>
closer = 'ncoghlan'
components = ['Interpreter Core']
creation = <Date 2011-03-26.03:36:45.133>
creator = 'ncoghlan'
dependencies = []
files = ['24214']
hgrepos = ['40', '107']
issue_num = 11682
keywords = ['patch']
message_count = 35.0
messages = ['132213', '134550', '134556', '134925', '139145', '139471', '139529', '140072', '140110', '140115', '143210', '143492', '143493', '143510', '144325', '144350', '144378', '145144', '146695', '147320', '147347', '147348', '147404', '148153', '148163', '148950', '148959', '148963', '148964', '148977', '151125', '151144', '151171', '151177', '152085']
nosy_count = 12.0
nosy_names = ['rhettinger', 'gcewing', 'ncoghlan', 'rndblnch', 'ron_adam', 'aronacher', 'eric.araujo', 'alex', 'rfk', 'asvetlov', 'zbysz', 'Yury.Selivanov']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue11682'
versions = ['Python 3.3']
Activity
ncoghlan commentedon Mar 26, 2011
Creating an issue to track efforts to forward port the PEP-380 reference implementation to Python 3.3.
merwok commentedon Apr 27, 2011
(“Create patch” does not work for some reason, I’ll report that to the metatracker in a few days if nobody does it first)
ncoghlan commentedon Apr 27, 2011
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.
rndblnch commentedon May 1, 2011
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
ncoghlan commentedon Jun 26, 2011
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
ncoghlan commentedon Jun 30, 2011
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.
rndblnch commentedon Jun 30, 2011
I've just updated the PEP-380-test patch to make it slightly simpler (still in golden output form though)
https://bitbucket.org/rndblnch/cpython-pep380/src/317eadf5e3e8/pep380-tests
ncoghlan commentedon Jul 9, 2011
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.
rndblnch commentedon Jul 11, 2011
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 :)
ncoghlan commentedon Jul 11, 2011
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 bpo-11816 as a dependency as some of the new tests will be easier to refactor once dis.get_opinfo() is available.
ncoghlan commentedon Aug 30, 2011
The PEP-380 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 bpo-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.
ncoghlan commentedon Sep 4, 2011
Uploaded a patch that should be complete. Note that my PEP-380 branch is based on my get_opinfo branch (see bpo-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 bpo-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.
ncoghlan commentedon Sep 4, 2011
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)
zbysz commentedon Sep 5, 2011
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.
10 remaining items
ncoghlan commentedon Nov 23, 2011
*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
ronadam commentedon Dec 7, 2011
There is a test for 'yield from' as a function argument without the extra parentheses.
You do need them in the case of a regular yield.
Shouldn't the same rule apply in both cases?
zbysz commentedon Dec 7, 2011
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.").
ncoghlan commentedon Dec 7, 2011
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).
ncoghlan commentedon Dec 7, 2011
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.
ronadam commentedon Dec 7, 2011
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.
The purpose of doing that is so I can do ...
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. :-)
ncoghlan commentedon Jan 12, 2012
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 :)
zbysz commentedon Jan 12, 2012
Some minor comments in http://bugs.python.org/review/11682/show.
ncoghlan commentedon Jan 13, 2012
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 :)
merwok commentedon Jan 13, 2012
Kudos!
ncoghlan commentedon Jan 27, 2012
Just noting that the PEP-380 integration branch is also available in the hg.python.org clone of my sandbox repo.