python / cpython

The Python programming language
https://www.python.org
Other
62.4k stars 29.96k forks source link

Better expose closure, generator & coroutine status of functions #68244

Open ncoghlan opened 9 years ago

ncoghlan commented 9 years ago
BPO 24056
Nosy @rhettinger, @terryjreedy, @mdickinson, @ncoghlan, @ezio-melotti, @bitdancer, @berkerpeksag, @serhiy-storchaka, @1st1
Files
  • issue24056.diff
  • issue24056_2.diff
  • 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: ```python assignee = None closed_at = None created_at = labels = ['interpreter-core', 'type-feature'] title = 'Better expose closure, generator & coroutine status of functions' updated_at = user = 'https://github.com/ncoghlan' ``` bugs.python.org fields: ```python activity = actor = 'petr.viktorin' assignee = 'none' closed = False closed_date = None closer = None components = ['Interpreter Core'] creation = creator = 'ncoghlan' dependencies = [] files = ['39224', '39461'] hgrepos = [] issue_num = 24056 keywords = ['patch'] message_count = 14.0 messages = ['241994', '242176', '242185', '242214', '242360', '242403', '242406', '243780', '243834', '243838', '243845', '243861', '243885', '243895'] nosy_count = 10.0 nosy_names = ['rhettinger', 'terry.reedy', 'mark.dickinson', 'ncoghlan', 'ezio.melotti', 'Arfrever', 'r.david.murray', 'berker.peksag', 'serhiy.storchaka', 'yselivanov'] pr_nums = [] priority = 'normal' resolution = None stage = 'patch review' status = 'open' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue24056' versions = ['Python 3.6'] ```

    ncoghlan commented 9 years ago

    From https://mail.python.org/pipermail/python-ideas/2015-April/033177.html, there are some additional details about functions that could be usefully exposed in the function repr, specifically whether or not it's a closure, and whether or not it's a generator function.

    Proposed display:

    <function f at 0x7f7dad9f7bf8 (closure)>
    <function f at 0x7f7dad9f7bf8 (generator)>
    <function f at 0x7f7dad9f7bf8 (closure,generator)>

    The Python level checks for the two flags:

    closure: f.__closure is not None generator: c.__code.co_flags & inspect.CO_GENERATOR

    Actual implementation would be in the C code at https://hg.python.org/cpython/file/default/Objects/funcobject.c#l569

    berkerpeksag commented 9 years ago

    Here is a patch with a test.

    mdickinson commented 9 years ago

    I can see that the generator information would be useful. What's the use-case for reporting that a function is a closure? I'm having trouble thinking of a case where it's useful to know that a function is a closure without also knowing which locals refer to cells.

    ncoghlan commented 9 years ago

    It's mostly pedagogical - similar to "normal functions" vs "generator functions", folks talk about functions and closures as different things, even though in Python a closure is just a normal function with one or more references to cells that were defined in outer scopes.

    Having that show up in the repr() then becomes a way of clarifying that some, but not all, Python function objects are closures, even though closures aren't represented as a distinct type.

    That difference also shows up in the bytecode that creates them (note the MAKE_FUNCTION vs MAKE_CLOSURE):

    >>> def outer():
    ...     x = 1
    ...     def inner_function():
    ...         pass
    ...     def inner_closure():
    ...         return x
    ... 
    >>> import dis
    >>> dis.dis(outer)
      2           0 LOAD_CONST               1 (1)
                  3 STORE_DEREF              0 (x)

    3 6 LOAD_CONST 2 (\<code object inner_function at 0x7fade75e5c90, file "\<stdin>", line 3>) 9 LOAD_CONST 3 ('outer.\<locals>.inner_function') 12 MAKE_FUNCTION 0 15 STORE_FAST 0 (inner_function)

    5 18 LOAD_CLOSURE 0 (x) 21 BUILD_TUPLE 1 24 LOAD_CONST 4 (\<code object inner_closure at 0x7fade75e5a50, file "\<stdin>", line 5>) 27 LOAD_CONST 5 ('outer.\<locals>.inner_closure') 30 MAKE_CLOSURE 0 33 STORE_FAST 1 (inner_closure) 36 LOAD_CONST 0 (None) 39 RETURN_VALUE

    One particular case where the distinction matters and is known to be genuinely confusing for new Python users is the late binding behaviour of closures:

    lambda: i # closure
    lambda i=i: i # not a closure
    terryjreedy commented 9 years ago

    Describing generator functions as such is a great idea. But how about \<generator function f at 0x7f7dad9f7bf8>

    Marking closure functions as such is a bit more subtle. However, there ia a real point that closure functions have a hidden input. If it is mutated or rebound, the function will not be deterministic with respect to its overt input arguments. Closure functions are similar to methods in this respect.

    Await functions (Guido's name choice as of today), if the PEP is approved, will also need to be identified as such. I propose a uniform format of no prefix, a single prefic or a (tuple) of prefixes.

    ncoghlan commented 9 years ago

    The main reason I suggest using the postfix parenthetical syntax is to make it clear that we're exposing "behavioural feature flags" for a single underlying type. A prefix syntax would make them look like distinct types, which would be misleading in a different way.

    bitdancer commented 9 years ago

    Although I like the look of the repr Terry proposes better, I agree with Nick: it would imply that the types were distinct, which they are not.

    1st1 commented 9 years ago

    Nick, Berker, please find an updated patch attached (with support for coroutines). Big +1 on the idea, BTW.

    1st1 commented 9 years ago

    Nick, Berker, a kind reminder -- please review the patch if we want to have it in 3.5.

    berkerpeksag commented 9 years ago

    I'm not the ideal candidate to review the second patch since I'm not familiar with the best practices of C :)

    serhiy-storchaka commented 9 years ago

    I like the look of the repr Terry proposes better. For generator objects the repr is either "\<coroutine object %S at %p>" or "\<generator object %S at %p>". "\<coroutine function %S at %p>" and "\<generator function %S at %p>" would be consistent with this. It also shows the relation and the difference between the generator function and the generator object.

    Yet one argument is that both terms "generator object" and "generator function" are searchable in the documentation.

    There are other precedences with exposing flags at the start of the repr. "\<built-in function %s>" and "\<built-in method %s of %s object at %p>", "\<unlocked %s object at %p>" and "\<locked %s object at %p>".

    rhettinger commented 9 years ago

    It's mostly pedagogical - similar to "normal functions" vs "generator functions",

    I see a need for this but object to calling it a "generator" rather than a "function that makes a generator" or "generator creating function" or somesuch. There is a huge semantic difference between the two.

    Another thought this that I'm not sure that a __repr should try usurp something that is the primary responsibility of a docstring or function annotation here. Whether a function call runs code and returns a value or whether it returns a generator is fundamental to what the function does. The usual job of the __repr is to tell what the object is. The usual job of a docstring or type annotation to the describe what is returned.

    Marking closure functions as such is a bit more subtle. However, there ia a real point that closure functions have a hidden input.

    I don't see a need for this and think it make cause more confusion than help. I try to teach that callables are all conceptually the same thing (something that has a __call method). It matters very little whether a callable is implemented as a closure or using a class with a __call method.

    So, put me down for -1 on this one.

    ncoghlan commented 9 years ago

    I don't think we should rush this one, especially as PEP-484 provides the possibility for tools (including educational tools) to infer the appropriate return types for generator and coroutine functions.

    Bumping the target version to 3.6 accordingly.

    ncoghlan commented 9 years ago

    I've also come to agree with Raymond that the repr may not be the best place for this additional information, and have updated the issue title accordingly.

    For example, as one possible alternative, we might be able to put something in the inspect module (e.g. "inspect.callable_info()") that's a higher level alternative to "dis.code_info()".

    The information displayed could potentially include:

    More controversially, it might include an inferred return type annotation when there's no explicit annotation to display (defaulting to "typing.Any", but potentially more explicit if it's possible to tell from the code object flags that calling it will return a Coroutine or Generator)