Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introspection generator and function closure state #57271

Closed
ncoghlan opened this issue Sep 29, 2011 · 18 comments
Closed

Introspection generator and function closure state #57271

ncoghlan opened this issue Sep 29, 2011 · 18 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@ncoghlan
Copy link
Contributor

ncoghlan commented Sep 29, 2011

BPO 13062
Nosy @ncoghlan, @meadori, @durban, @ericsnowcurrently
Files
  • issue13062.patch: v1 patch against tip (3.3.0a0)
  • issue13062-2.patch: v2 patch against tip (3.3.0a0)
  • issue13062-3.patch: v3 patch against tip (3.3.0a0)
  • issue13062-combined.diff: Combined patch for getclosurevars and getgeneratorlocals
  • issue13062-getclosurevars.diff: Just the getclosurevars changes
  • 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-06-23.09:40:22.753>
    created_at = <Date 2011-09-29.17:43:09.969>
    labels = ['type-feature', 'library']
    title = 'Introspection generator and function closure state'
    updated_at = <Date 2012-06-23.09:40:22.685>
    user = 'https://github.com/ncoghlan'

    bugs.python.org fields:

    activity = <Date 2012-06-23.09:40:22.685>
    actor = 'python-dev'
    assignee = 'ncoghlan'
    closed = True
    closed_date = <Date 2012-06-23.09:40:22.753>
    closer = 'python-dev'
    components = ['Library (Lib)']
    creation = <Date 2011-09-29.17:43:09.969>
    creator = 'ncoghlan'
    dependencies = []
    files = ['23303', '23356', '23368', '26103', '26104']
    hgrepos = []
    issue_num = 13062
    keywords = ['patch']
    message_count = 18.0
    messages = ['144606', '144636', '144638', '144640', '144641', '144798', '144799', '144801', '145264', '145296', '145300', '145308', '146277', '162671', '162707', '163558', '163562', '163564']
    nosy_count = 7.0
    nosy_names = ['ncoghlan', 'ron_adam', 'meador.inge', 'daniel.urban', 'Yury.Selivanov', 'python-dev', 'eric.snow']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue13062'
    versions = ['Python 3.3']

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Sep 29, 2011

    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).

    @ncoghlan ncoghlan added the type-feature A feature request or enhancement label Sep 29, 2011
    @meadori meadori added the stdlib Python modules in the Lib dir label Sep 29, 2011
    @meadori
    Copy link
    Member

    meadori commented Sep 29, 2011

    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>}

    ?

    @ericsnowcurrently
    Copy link
    Member

    ericsnowcurrently commented Sep 29, 2011

    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 */

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Sep 29, 2011

    Yep, that looks right to me. The eval loop then references those cells from the frame object during execution.

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Sep 29, 2011

    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__)}

    @meadori
    Copy link
    Member

    meadori commented Oct 3, 2011

    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.

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Oct 3, 2011

    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.

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Oct 3, 2011

    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.

    @meadori
    Copy link
    Member

    meadori commented Oct 9, 2011

    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.

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Oct 10, 2011

    No, the naming problem had occurred to me as well. Given the 'vars' builtin, perhaps 'getclosurevars' would do as the name?

    @meadori
    Copy link
    Member

    meadori commented Oct 10, 2011

    perhaps 'getclosurevars' would do as the name?

    I like vars. Updated patch attached.

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Oct 10, 2011

    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.

    @meadori
    Copy link
    Member

    meadori commented Oct 24, 2011

    Nick, the revised definition of 'getclosurevars' seems reasonable to me.
    I will cut a new patch this week.

    @ncoghlan ncoghlan self-assigned this Jun 12, 2012
    @meadori
    Copy link
    Member

    meadori commented Jun 12, 2012

    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.

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Jun 13, 2012

    Meador: I probably won't get to this until the weekend, so go ahead and update the patch if you have time.

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Jun 23, 2012

    Attached patch implements both new functions, but I'm going to drop getgeneratorlocals for now and move that idea to a new issue.

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Jun 23, 2012

    I created bpo-15153 to cover getgeneratorlocals. Attached patch is just for record keeping purposes - I'll be committing this change shortly.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 23, 2012

    New changeset 487fe648de56 by Nick Coghlan in branch 'default':
    Close bpo-13062: Add inspect.getclosurevars to simplify testing stateful closures
    http://hg.python.org/cpython/rev/487fe648de56

    @python-dev python-dev mannequin closed this as completed Jun 23, 2012
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants