classification
Title: Introspection generator and function closure state
Type: enhancement Stage: committed/rejected
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ncoghlan Nosy List: Yury.Selivanov, daniel.urban, eric.snow, meador.inge, ncoghlan, python-dev, ron_adam
Priority: normal Keywords: patch

Created on 2011-09-29 17:43 by ncoghlan, last changed 2012-06-23 09:40 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
issue13062.patch meador.inge, 2011-10-03 01:36 v1 patch against tip (3.3.0a0) review
issue13062-2.patch meador.inge, 2011-10-09 15:29 v2 patch against tip (3.3.0a0) review
issue13062-3.patch meador.inge, 2011-10-10 14:12 v3 patch against tip (3.3.0a0) review
issue13062-combined.diff ncoghlan, 2012-06-23 09:14 Combined patch for getclosurevars and getgeneratorlocals review
issue13062-getclosurevars.diff ncoghlan, 2012-06-23 09:24 Just the getclosurevars changes review
Messages (18)
msg144606 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-09-29 17:43
Based on the python-ideas thread about closures, I realised there are two features the inspect module could offer to greatly simplify some aspects of testing closure and generator behaviour:

  inspect.getclosure(func)
    Returns a dictionary mapping closure references from the supplied function to their current values.

  inspect.getgeneratorlocals(generator)
    Returns the same result as would be reported by calling locals() in the generator's frame of execution

The former would just involve syncing up the names on the code object with the cell references on the function object, while the latter would be equivalent to doing generator.gi_frame.f_locals with some nice error checking for when the generator's frame is already gone (or the supplied object isn't a generator iterator).
msg144636 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2011-09-29 20:40
I'll take a shot and writing a patch for this one.  Nick, are the 
elements in 'co_freevars' and '__closures__' always expected to match
up?  In other words, is the 'closure' function below always expected to 
work (simplified; no error checking):

>>> def make_adder(x):
...     def add(y):
...         return x + y
...     return add
... 
>>> def curry(func, arg1):
...     return lambda arg2: func(arg1, arg2)
... 
>>> def less_than(a, b):
...     return a < b
... 
>>> greater_than_five = curry(less_than, 5)
>>> def closure(func):
...     vars = [var for var in func.__code__.co_freevars]
...     values = [cell.cell_contents for cell in func.__closure__]
...     return dict(zip(vars, values))
...
>>> inc = make_adder(1)
>>> print(closure(inc))
{'x': 1}
>>> print(closure(greater_than_five))
{'arg1': 5, 'func': <function less_than at 0xb74c6924>}

?
msg144638 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2011-09-29 20:54
See:
http://hg.python.org/cpython/file/default/Include/funcobject.h#l34
http://hg.python.org/cpython/file/default/Objects/funcobject.c#l454


   454 /* func_new() maintains the following invariants for closures.  The
   455    closure must correspond to the free variables of the code object.
   456 
   457    if len(code.co_freevars) == 0:
   458        closure = NULL
   459    else:
   460        len(closure) == len(code.co_freevars)
   461    for every elt in closure, type(elt) == cell
   462 */
msg144640 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-09-29 21:14
Yep, that looks right to me. The eval loop then references those cells from the frame object during execution.
msg144641 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-09-29 21:18
Huh, I didn't actually realise getclosure() could be written as a one liner until seeing Meador's version above:

  {var : cell.cell_contents for var, cell in zip(func.__code__.co_freevars, func.__closure__)}
msg144798 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2011-10-03 01:36
Here is a first cut at a patch.  There is one slight deviation from the original spec:

> some nice error checking for when the generator's frame is already gone > (or the supplied object isn't a generator iterator).

The attached patch returns empty mappings for these cases.  I can easily
add the error checks, but in what cases is it useful to know *exactly*
why a mapping could not be created?  Having an empty mapping for all 
invalid cases is simpler and seems more robust.
msg144799 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-10-03 02:02
Because a generator can legitimately have no locals:

>>> def gen():
...     yield 1
... 
>>> g = gen()
>>> g.gi_frame.f_locals
{}

Errors should be reported as exceptions - AttributeError or TypeError if there's no gi_frame and then ValueError or RuntimeError if gi_frame is None.
msg144801 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-10-03 02:12
The function case is simpler - AttributeError or TypeError if there's no __closure__ attribute, empty mapping if there's no closure.

I've also changed my mind on the "no frame" generator case - since that mapping will evolve over time as the generator executes anyway, the empty mapping accurately reflects the "no locals currently defined" that applies when the generator either hasn't been started yet or has finished. People can use getgeneratorstate() to find that information if they need to know.
msg145264 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2011-10-09 15:29
Here is an updated patch with error handling.  One other thought is that 
'getclosure' should be called something like 'getclosureenv' since 
technically a closure is a function plus its environment and our 
implementation only returns the environment.  But that may be converging 
on pedantic.
msg145296 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-10-10 12:18
No, the naming problem had occurred to me as well. Given the 'vars' builtin, perhaps 'getclosurevars' would do as the name?
msg145300 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2011-10-10 14:12
> perhaps 'getclosurevars' would do as the name?

I like vars.  Updated patch attached.
msg145308 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-10-10 16:00
In reviewing Meador's patch (which otherwise looks pretty good), I had a thought about the functionality and signature of getclosurevars().

Currently, it equates "closure" to "nonlocal scope", which isn't really true - the function's closure is really the current binding of *all* of its free variables, and that includes globals and builtins in addition to the lexically scoped variables from outer scopes.

So what do people think about this signature:

  ClosureVars = namedtuple("ClosureVars", "nonlocals globals builtins unbound")
  def getclosurevars(func):
    """Returns a named tuple of dictionaries of the current nonlocal, global and builtin references as seen by the body of the function. A final set of unbound names is also provided."""
    # figure out nonlocal_vars (current impl)
    # figure out global_vars (try looking up names in f_globals)
    # figure out builtin_vars (try looking up names in builtins)
    # any leftover names go in unbound_vars
    return ClosureVars(nonlocal_vars, global_vars, builtin_vars, unbound_vars)

Also, something that just occurred to me is that getclosurevars() should work for already instantiated generator iterators as well as generator functions, so the current typecheck may need to be made a bit more flexible.
msg146277 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2011-10-24 04:02
Nick, the revised definition of 'getclosurevars' seems reasonable to me.
I will cut a new patch this week.
msg162671 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2012-06-12 13:24
I didn't get around to updating my patch with Nick's comments yet.

Nick, the v3 patch I have attached still applies.  I am happy to update it per your comments (promptly this time) or you can take it over.  Whichever.
msg162707 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-06-13 13:40
Meador: I probably won't get to this until the weekend, so go ahead and update the patch if you have time.
msg163558 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-06-23 09:14
Attached patch implements both new functions, but I'm going to drop getgeneratorlocals for now and move that idea to a new issue.
msg163562 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-06-23 09:24
I created #15153 to cover getgeneratorlocals. Attached patch is just for record keeping purposes - I'll be committing this change shortly.
msg163564 - (view) Author: Roundup Robot (python-dev) Date: 2012-06-23 09:40
New changeset 487fe648de56 by Nick Coghlan in branch 'default':
Close #13062: Add inspect.getclosurevars to simplify testing stateful closures
http://hg.python.org/cpython/rev/487fe648de56
History
Date User Action Args
2012-06-23 09:40:22python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg163564

resolution: fixed
stage: patch review -> committed/rejected
2012-06-23 09:24:40ncoghlansetfiles: + issue13062-getclosurevars.diff

messages: + msg163562
2012-06-23 09:14:39ncoghlansetfiles: + issue13062-combined.diff

messages: + msg163558
2012-06-13 13:40:17ncoghlansetmessages: + msg162707
2012-06-12 13:24:59meador.ingesetmessages: + msg162671
2012-06-12 12:05:35ncoghlansetassignee: ncoghlan
2011-10-24 04:02:31meador.ingesetmessages: + msg146277
2011-10-10 20:20:02ron_adamsetnosy: + ron_adam
2011-10-10 16:00:18ncoghlansetmessages: + msg145308
2011-10-10 14:12:45meador.ingesetfiles: + issue13062-3.patch

messages: + msg145300
2011-10-10 12:18:45ncoghlansetmessages: + msg145296
2011-10-09 15:29:15meador.ingesetfiles: + issue13062-2.patch

messages: + msg145264
2011-10-03 02:12:03ncoghlansetmessages: + msg144801
2011-10-03 02:02:59ncoghlansetmessages: + msg144799
2011-10-03 01:36:05meador.ingesetfiles: + issue13062.patch
keywords: + patch
messages: + msg144798

stage: needs patch -> patch review
2011-09-30 16:25:45daniel.urbansetnosy: + daniel.urban
2011-09-29 21:18:50ncoghlansetmessages: + msg144641
2011-09-29 21:14:23ncoghlansetmessages: + msg144640
2011-09-29 20:54:03eric.snowsetmessages: + msg144638
2011-09-29 20:40:30meador.ingesetmessages: + msg144636
2011-09-29 19:12:33eric.snowsetnosy: + eric.snow
2011-09-29 19:00:42Yury.Selivanovsetnosy: + Yury.Selivanov
2011-09-29 18:29:37meador.ingesetnosy: + meador.inge
2011-09-29 18:29:14meador.ingesetstage: needs patch
components: + Library (Lib)
versions: + Python 3.3
2011-09-29 17:43:09ncoghlancreate