PEP 362 reference implementation for 3.3

[Jim.Jewett]

http://bugs.python.org/review/15008/diff/5143/Lib/inspect.py
File Lib/inspect.py (right):

http://bugs.python.org/review/15008/diff/5143/Lib/inspect.py#newcode1786
Lib/inspect.py:1786: def format(self, *, format_name=str,
Should this be a separate class, so that it is easier to override individual
format functions, without having to redo the whole logic?

[christian.heimes]

As explained on the python-dev mailing list.

Christian

http://bugs.python.org/review/15008/diff/5143/Objects/methodobject.c
File Objects/methodobject.c (right):

http://bugs.python.org/review/15008/diff/5143/Objects/methodobject.c#newcode224
Objects/methodobject.c:224: {"__signature__", T_OBJECT,     OFF(m_signature),
0},
__signature__ must be PY_WRITE_RESTRICTED. You mustn't add a mutable member to 
an object which is shared across subinterpreters.

If you want mutable attributes than you have to store then (for example) in the
current PyInterpreterState and use __qualname__ or its id() as identifier. This
also reduces the memory cost, as you just need one additional pointer per state
instead for each function.

[larry]

http://bugs.python.org/review/15008/diff/5143/Objects/methodobject.c
File Objects/methodobject.c (right):

http://bugs.python.org/review/15008/diff/5143/Objects/methodobject.c#newcode224
Objects/methodobject.c:224: {"__signature__", T_OBJECT,     OFF(m_signature),
0},
The object that the PEP proposes to be stored in __signature__ is itself
mutable--that's why the PEP talks about making a deep copy.  So
PY_WRITE_RESTRICTED is insufficient.  We'd need to have a completely immutable
form of Signature.  We'd then have to initialize those as part of the
interpreter.  This is all completely hypothetical; we have no hand-coded
signatures right now, and given that "is_implemented" is gonna get the axe I'm
not planning on writing any.

If Christian is right about this--which I assume he is--then we should remove
__signature__ completely.

Christian: are modules implemented in Python shared across instances?  If they
aren't, it would be straightforward to implement a very simple cache directly in
sys/inspect.py.

[christian.heimes]

On 2012/06/16 01:56:53, larry wrote:
> Christian: are modules implemented in Python shared across instances?  If they
> aren't, it would be straightforward to implement a very simple cache directly
in
> sys/inspect.py.

No, modules aren't shared across subinterpreter states. It's tricky and it takes
a while to understand the concept and implementation details. Every
subinterpreter has its own PyInterpreterState, even the 'main' interpreter.
Every Python thread belongs to exactly one interpreter state.

Each PyInterpreterState has its own sys module instances. The sys module
contains the loaded modules in sys.modules. If you import a module from another
subinterpreter it's loaded, parsed and created a second time.

At the moment I can recall how it is exactly done for C modules. Martin's
improved API for C extensions should do the trick for global module states.
However types and and function objects are true singletons for the entire
process space. Objects like None, True, False, small ints etc. are also
singletons and shared across subinterpreters.

[Yury.Selivanov]

http://bugs.python.org/review/15008/diff/5143/Lib/inspect.py
File Lib/inspect.py (right):

http://bugs.python.org/review/15008/diff/5143/Lib/inspect.py#newcode1786
Lib/inspect.py:1786: def format(self, *, format_name=str,
On 2012/06/15 21:00:04, Jim.Jewett wrote:
> Should this be a separate class, so that it is easier to override individual
> format functions, without having to redo the whole logic?

Good idea. I like it.

http://bugs.python.org/review/15008/diff/5143/Objects/methodobject.c
File Objects/methodobject.c (right):

http://bugs.python.org/review/15008/diff/5143/Objects/methodobject.c#newcode224
Objects/methodobject.c:224: {"__signature__", T_OBJECT,     OFF(m_signature),
0},
On 2012/06/16 01:56:53, larry wrote:
> The object that the PEP proposes to be stored in __signature__ is itself
> mutable--that's why the PEP talks about making a deep copy.  So
> PY_WRITE_RESTRICTED is insufficient.  We'd need to have a completely immutable
> form of Signature.  We'd then have to initialize those as part of the
> interpreter.  This is all completely hypothetical; we have no hand-coded
> signatures right now, and given that "is_implemented" is gonna get the axe I'm
> not planning on writing any.
> 
> If Christian is right about this--which I assume he is--then we should remove
> __signature__ completely.
> 
> Christian: are modules implemented in Python shared across instances?  If they
> aren't, it would be straightforward to implement a very simple cache directly
in
> sys/inspect.py.

Let's just remove it.  I don't want to start discussions on how to make things
better on the 'C' side, as the PEP surely won't make it to 3.3 then.

[Jim.Jewett]

http://bugs.python.org/review/15008/diff/5159/Lib/inspect.py
File Lib/inspect.py (right):

http://bugs.python.org/review/15008/diff/5159/Lib/inspect.py#newcode1824
Lib/inspect.py:1824: if param.kind == _VAR_POSITIONAL:
Is there a reason this isn't combined with the same condition 5 lines up?  It
would be OK to leave the _KEYWORD_ONLY test an elif, since the kinds are
exclusive.

[Yury.Selivanov]

http://bugs.python.org/review/15008/diff/5159/Lib/inspect.py
File Lib/inspect.py (right):

http://bugs.python.org/review/15008/diff/5159/Lib/inspect.py#newcode1824
Lib/inspect.py:1824: if param.kind == _VAR_POSITIONAL:
On 2012/06/19 05:44:00, Jim.Jewett wrote:
> Is there a reason this isn't combined with the same condition 5 lines up?  It
> would be OK to leave the _KEYWORD_ONLY test an elif, since the kinds are
> exclusive.

Just wanted to separate the formatting logic from the keyword-only separator
logic, as initially this code was an example in the PEP.  I think I'll just
combine them, as you suggest.

Thanks!

[Yury.Selivanov]

http://bugs.python.org/review/15008/diff/5159/Lib/inspect.py
File Lib/inspect.py (right):

http://bugs.python.org/review/15008/diff/5159/Lib/inspect.py#newcode1824
Lib/inspect.py:1824: if param.kind == _VAR_POSITIONAL:
On 2012/06/19 17:00:59, Yury.Selivanov wrote:
> On 2012/06/19 05:44:00, Jim.Jewett wrote:
> > Is there a reason this isn't combined with the same condition 5 lines up? 
It
> > would be OK to leave the _KEYWORD_ONLY test an elif, since the kinds are
> > exclusive.
> 
> Just wanted to separate the formatting logic from the keyword-only separator
> logic, as initially this code was an example in the PEP.  I think I'll just
> combine them, as you suggest.
> 
> Thanks!

https://bitbucket.org/1st1/cpython/changeset/bf009eb6e1b4

[ezio.melotti]

http://bugs.python.org/review/15008/diff/5161/Lib/inspect.py
File Lib/inspect.py (right):

http://bugs.python.org/review/15008/diff/5161/Lib/inspect.py#newcode1325
Lib/inspect.py:1325: and not getattr(param, '__partial_kwarg__', False):
Parentheses are preferred to '\', the 'and' usually goes on the first line, and
the second line should be indented with 'param', i.e.:
elif (param.kind not in (_VAR_KEYWORD, _VAR_POSITIONAL) and
      not getattr(param, '__partial_kwarg__', False)):

http://bugs.python.org/review/15008/diff/5161/Lib/inspect.py#newcode1368
Lib/inspect.py:1368: raise ValueError('no signature found for builtin ' \
The '\' is not necessary here.

http://bugs.python.org/review/15008/diff/5161/Lib/inspect.py#newcode1375
Lib/inspect.py:1375: '''A private marker - used in Parameter'''
I usually see _void = object() in these cases, but I guess that both works.

http://bugs.python.org/review/15008/diff/5161/Lib/inspect.py#newcode1417
Lib/inspect.py:1417: return '<{} at 0x{:x}
{!r}>'.format(self.__class__.__name__,
0x{:x} is equivalent to {:#x}

http://bugs.python.org/review/15008/diff/5161/Lib/inspect.py#newcode1433
Lib/inspect.py:1433: getattr(other, 'annotation', _void)
I would do something like this here:
return (issubclass(other.__class__, Parameter) and
        self.name == other.name and
        self.kind == other.kind and
        (getattr(self, 'default', _void) ==
         getattr(other, 'default', _void)) and
        (getattr(self, 'annotation', _void) ==
         getattr(other, 'annotation', _void)))

http://bugs.python.org/review/15008/diff/5161/Lib/inspect.py#newcode1443
Lib/inspect.py:1443: _VAR_KEYWORD            = Parameter.VAR_KEYWORD
Why not defining these once before Parameter and then use these constants in
Parameter too?

http://bugs.python.org/review/15008/diff/5161/Lib/inspect.py#newcode1641
Lib/inspect.py:1641: sig.parameters = OrderedDict((name, param.__copy__()) \
'\' not necessary

http://bugs.python.org/review/15008/diff/5161/Lib/test/test_inspect.py
File Lib/test/test_inspect.py (right):

http://bugs.python.org/review/15008/diff/5161/Lib/test/test_inspect.py#newcod...
Lib/test/test_inspect.py:1195: def test_signature_on_wkwonly(self):
wkw?

http://bugs.python.org/review/15008/diff/5161/Lib/test/test_inspect.py#newcod...
Lib/test/test_inspect.py:1211: ...))
To avoid weird indentations I usually do:
expected = (
    ((('a', ..., ..., "positional_or_ keyword"),
     ('b', 10, 'foo', "positional_or _keyword"),
     ...
)
self.assertEqual(self.signature(test), expected)

[Yury.Selivanov]

http://bugs.python.org/review/15008/diff/5161/Lib/inspect.py
File Lib/inspect.py (right):

http://bugs.python.org/review/15008/diff/5161/Lib/inspect.py#newcode1325
Lib/inspect.py:1325: and not getattr(param, '__partial_kwarg__', False):
On 2012/06/19 20:34:45, ezio.melotti wrote:
> Parentheses are preferred to '\', the 'and' usually goes on the first line,
and
> the second line should be indented with 'param', i.e.:
> elif (param.kind not in (_VAR_KEYWORD, _VAR_POSITIONAL) and
>       not getattr(param, '__partial_kwarg__', False)):

I don't see this in PEP8.  Is it some secret stdlib rule?  Or it's your personal
preference?

http://bugs.python.org/review/15008/diff/5161/Lib/inspect.py#newcode1368
Lib/inspect.py:1368: raise ValueError('no signature found for builtin ' \
On 2012/06/19 20:34:45, ezio.melotti wrote:
> The '\' is not necessary here.
OK

http://bugs.python.org/review/15008/diff/5161/Lib/inspect.py#newcode1417
Lib/inspect.py:1417: return '<{} at 0x{:x}
{!r}>'.format(self.__class__.__name__,
On 2012/06/19 20:34:45, ezio.melotti wrote:
> 0x{:x} is equivalent to {:#x}

OK

http://bugs.python.org/review/15008/diff/5161/Lib/inspect.py#newcode1433
Lib/inspect.py:1433: getattr(other, 'annotation', _void)
On 2012/06/19 20:34:45, ezio.melotti wrote:
> I would do something like this here:
> return (issubclass(other.__class__, Parameter) and
>         self.name == other.name and
>         self.kind == other.kind and
>         (getattr(self, 'default', _void) ==
>          getattr(other, 'default', _void)) and
>         (getattr(self, 'annotation', _void) ==
>          getattr(other, 'annotation', _void)))

OK. It's a bit more readable in parenthesis.

http://bugs.python.org/review/15008/diff/5161/Lib/inspect.py#newcode1443
Lib/inspect.py:1443: _VAR_KEYWORD            = Parameter.VAR_KEYWORD
On 2012/06/19 20:34:45, ezio.melotti wrote:
> Why not defining these once before Parameter and then use these constants in
> Parameter too?

Will it make any difference?

http://bugs.python.org/review/15008/diff/5161/Lib/inspect.py#newcode1641
Lib/inspect.py:1641: sig.parameters = OrderedDict((name, param.__copy__()) \
On 2012/06/19 20:34:46, ezio.melotti wrote:
> '\' not necessary

OK

[ezio.melotti]

On 2012/06/19 20:47:10, Yury.Selivanov wrote:
> http://bugs.python.org/review/15008/diff/5161/Lib/inspect.py
> File Lib/inspect.py (right):
> 
> http://bugs.python.org/review/15008/diff/5161/Lib/inspect.py#newcode1325
> Lib/inspect.py:1325: and not getattr(param, '__partial_kwarg__', False):
> On 2012/06/19 20:34:45, ezio.melotti wrote:
> > Parentheses are preferred to '\', the 'and' usually goes on the first line,
> and
> > the second line should be indented with 'param', i.e.:
> > elif (param.kind not in (_VAR_KEYWORD, _VAR_POSITIONAL) and
> >       not getattr(param, '__partial_kwarg__', False)):
> 
> I don't see this in PEP8.  Is it some secret stdlib rule?  Or it's your
personal
> preference?
>
That's how I do it, but it's also written in the pep8:

http://www.python.org/dev/peps/pep-0008/#maximum-line-length
"""The preferred way of wrapping long lines is by using Python's implied line
continuation inside parentheses, brackets and braces. Long lines can be broken
over multiple lines by wrapping expressions in parentheses. These should be used
in preference to using a backslash for line continuation. Make sure to indent
the continued line appropriately. The preferred place to break around a binary
operator is after the operator, not before it."""


> http://bugs.python.org/review/15008/diff/5161/Lib/inspect.py#newcode1368
> Lib/inspect.py:1368: raise ValueError('no signature found for builtin ' \
> On 2012/06/19 20:34:45, ezio.melotti wrote:
> > The '\' is not necessary here.
> OK
> 

FTR there are a few other places where it's not necessary, I haven't listed them
all.
In general I (almost) never use the '\'.

> http://bugs.python.org/review/15008/diff/5161/Lib/inspect.py#newcode1417
> Lib/inspect.py:1417: return '<{} at 0x{:x}
> {!r}>'.format(self.__class__.__name__,
> On 2012/06/19 20:34:45, ezio.melotti wrote:
> > 0x{:x} is equivalent to {:#x}
> 
> OK
> 
> http://bugs.python.org/review/15008/diff/5161/Lib/inspect.py#newcode1433
> Lib/inspect.py:1433: getattr(other, 'annotation', _void)
> On 2012/06/19 20:34:45, ezio.melotti wrote:
> > I would do something like this here:
> > return (issubclass(other.__class__, Parameter) and
> >         self.name == other.name and
> >         self.kind == other.kind and
> >         (getattr(self, 'default', _void) ==
> >          getattr(other, 'default', _void)) and
> >         (getattr(self, 'annotation', _void) ==
> >          getattr(other, 'annotation', _void)))
> 
> OK. It's a bit more readable in parenthesis.
> 
> http://bugs.python.org/review/15008/diff/5161/Lib/inspect.py#newcode1443
> Lib/inspect.py:1443: _VAR_KEYWORD            = Parameter.VAR_KEYWORD
> On 2012/06/19 20:34:45, ezio.melotti wrote:
> > Why not defining these once before Parameter and then use these constants in
> > Parameter too?
> 
> Will it make any difference?
> 

If the user needs to access them from the class (i.e. they must be class
attributes) what you did (or the opposite) is ok, otherwise I would just define
them once as module-level _private constants.

> http://bugs.python.org/review/15008/diff/5161/Lib/inspect.py#newcode1641
> Lib/inspect.py:1641: sig.parameters = OrderedDict((name, param.__copy__()) \
> On 2012/06/19 20:34:46, ezio.melotti wrote:
> > '\' not necessary
> 
> OK

[Jim.Jewett]

http://bugs.python.org/review/15008/diff/5162/Lib/inspect.py
File Lib/inspect.py (right):

http://bugs.python.org/review/15008/diff/5162/Lib/inspect.py#newcode1471
Lib/inspect.py:1471: # Is this has a default value that was assigned to it
I think "Is this has a default value" should be "Does this have a default value"

http://bugs.python.org/review/15008/diff/5162/Lib/inspect.py#newcode1512
Lib/inspect.py:1512: if not kwargs_started and not partial:
I think this "and not partial" is now redundant because of partial forcing
kwargs_started in lines 1504-1506.

[Yury.Selivanov]

http://bugs.python.org/review/15008/diff/5162/Lib/inspect.py
File Lib/inspect.py (right):

http://bugs.python.org/review/15008/diff/5162/Lib/inspect.py#newcode1471
Lib/inspect.py:1471: # Is this has a default value that was assigned to it
On 2012/06/20 05:31:59, Jim.Jewett wrote:
> I think "Is this has a default value" should be "Does this have a default
value"

You're right.

http://bugs.python.org/review/15008/diff/5162/Lib/inspect.py#newcode1512
Lib/inspect.py:1512: if not kwargs_started and not partial:
On 2012/06/20 05:31:59, Jim.Jewett wrote:
> I think this "and not partial" is now redundant because of partial forcing
> kwargs_started in lines 1504-1506. 

Yep. Will fix this.

[berkerpeksag]

http://bugs.python.org/review/15008/diff/5175/Lib/inspect.py
File Lib/inspect.py (right):

http://bugs.python.org/review/15008/diff/5175/Lib/inspect.py#newcode1257
Lib/inspect.py:1257: '''Get a signature object for the passed callable.'''
Minor nit: ''' -> """

See: http://www.python.org/dev/peps/pep-0008/#documentation-strings