classification
Title: __qualname__ is not present on builtin methods and functions
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.3
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: gvanrossum, jcea, meador.inge, pitrou, python-dev, sbt
Priority: high Keywords: patch

Created on 2011-12-11 04:27 by meador.inge, last changed 2011-12-23 11:42 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
descr_qualname.patch sbt, 2011-12-11 13:30 review
descr_qualname.patch sbt, 2011-12-11 14:33
descr_qualname.patch sbt, 2011-12-12 00:03
method_qualname.patch sbt, 2011-12-12 11:49
descr_qualname.patch sbt, 2011-12-12 11:56
method_qualname.patch sbt, 2011-12-12 14:19
method_qualname.patch sbt, 2011-12-13 13:14
method_qualname.patch sbt, 2011-12-21 16:31
Messages (19)
msg149209 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2011-12-11 04:27
I was recently experimenting with the new PEP 3155 '__qualname__ implementation and noticed that '__qualname__' is not present on builtin methods and functions:

[meadori@motherbrain cpython]$ ./python 
Python 3.3.0a0 (default:aab45b904141+, Dec 10 2011, 14:53:54) 
[GCC 4.6.2 20111027 (Red Hat 4.6.2-1)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> max.__qualname__
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'builtin_function_or_method' object has no attribute '__qualname__'
>>> str.strip.__qualname__
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'method_descriptor' object has no attribute '__qualname__'

I will work up a patch.
msg149225 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2011-12-11 13:30
I already have a patch for the descriptor types which lazily calculates the __qualname__.  However test.test_sys also needs fixing because it  tests that these types have expected sizes.

I have not got round to builtin_function_or_method though.
msg149226 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2011-12-11 14:33
Updated patch which fixes test.test_sys.SizeofTest.  (It also adds __qualname__ to member descriptors and getset descriptors.)
msg149235 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-12-11 18:32
The patch should add some tests for the added functionality (I'm not sure where, but perhaps test_descr is a good location).

Also, just a nitpick, you can use _PyObject_GetAttrId and the _Py_IDENTIFIER macro instead of interning the "__qualname__" string yourself.

Note that extension (non-builtin) types will need to have their __qualname__ fixed before their methods' __qualname__ is usable:

>>> collections.deque.__qualname__
'deque'
msg149238 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2011-12-11 19:04
> Note that extension (non-builtin) types will need to have their 
> __qualname__ fixed before their methods' __qualname__ is usable:
>
> >>> collections.deque.__qualname__
> 'deque'

I'm confused.  Isn't that the expected behaviour?  Since the deque class is not nested inside another class or function, __qualname__ should be the same as __name__, shouldn't it?
msg149240 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-12-11 19:11
> > Note that extension (non-builtin) types will need to have their 
> > __qualname__ fixed before their methods' __qualname__ is usable:
> >
> > >>> collections.deque.__qualname__
> > 'deque'
> 
> I'm confused.  Isn't that the expected behaviour?  Since the deque
> class is not nested inside another class or function, __qualname__
> should be the same as __name__, shouldn't it?

Uh, yes, my bad.
msg149262 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2011-12-12 00:03
New version of the patch with tests and using _Py_IDENTIFIER.
msg149286 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-12-12 11:05
Ok, a couple of further (minor) issues:
- I don't think AssertionError is the right exception type. TypeError should be used when a type mismatches (e.g. "not an unicode object");
- you don't need to check for d_type being NULL, since other methods don't;
- if type_qualname == NULL, the original error should be retained.

Otherwise, looks good, thank you.
msg149291 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2011-12-12 11:49
Patch which add __qualname__ to builtin_function_or_method.  Note that I had to make a builtin staticmethod have __self__ be the type instead of None.
msg149293 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2011-12-12 11:56
> Ok, a couple of further (minor) issues:
> - I don't think AssertionError is the right exception type. TypeError 
> should be used when a type mismatches (e.g. "not an unicode object");
> - you don't need to check for d_type being NULL, since other methods don't;
> - if type_qualname == NULL, the original error should be retained.

Fixed.  (I suspect, though, that caching the value of __qualname__ is an unnecessary optimisation.)
msg149296 - (view) Author: Roundup Robot (python-dev) Date: 2011-12-12 12:47
New changeset 24238e89f938 by Antoine Pitrou in branch 'default':
Issue #13577: various kinds of descriptors now have a __qualname__ attribute.
http://hg.python.org/cpython/rev/24238e89f938
msg149299 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-12-12 13:27
About method_qualname.patch:

- apparently you forgot to add BuiltinFunctionPropertiesTest in test_main()?

- a static method keeps a reference to the type: I think it's ok, although I'm not sure about the consequences (Guido, would you have an idea?)

- about "XXX Note that it is not possible to use __qualname__ to distinguish a builtin bound instance method from its unbound equivalent", I don't think it's a problem. People can e.g. check for __self__:

>>> list.append.__self__
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'method_descriptor' object has no attribute '__self__'
>>> list().append.__self__
[]
msg149302 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2011-12-12 14:19
> - apparently you forgot to add BuiltinFunctionPropertiesTest in 
> test_main()?

Yes.  Fixed.

> - a static method keeps a reference to the type: I think it's ok, although 
> I'm not sure about the consequences (Guido, would you have an idea?)

Interestingly, in the stdlib METH_STATIC is only used by (str|bytes|bytearray).maketrans and _multiprocessing.win32.*.
msg149379 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2011-12-13 05:57
For the most part this looks OK, but I am not sure about this hunk:

diff --git a/Objects/typeobject.c b/Objects/typeobject.c
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -3724,7 +3724,7 @@ add_methods(PyTypeObject *type, PyMethod
             descr = PyDescr_NewClassMethod(type, meth);
         }
         else if (meth->ml_flags & METH_STATIC) {
-            PyObject *cfunc = PyCFunction_New(meth, NULL);
+            PyObject *cfunc = PyCFunction_New(meth, (PyObject*)type);
             if (cfunc == NULL)
                 return -1;
             descr = PyStaticMethod_New(cfunc);

That may be a breaking change as existing code may rely on the 'None' behavior.  In 
fact, this causes a unit test to fail with the patch applied:

[meadori@motherbrain cpython]$ ./python -m test test_descr
[1/1] test_descr
test test_descr failed -- Traceback (most recent call last):
  File "/home/meadori/src/python/cpython/Lib/test/test_descr.py", line 1485, in test_staticmethods_in_c
    self.assertEqual(x, None)
AssertionError: <class 'xxsubtype.spamlist'> != None

sbt, have you been running the test suite before submitting patches?  If not, then
please do.
msg149385 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2011-12-13 13:14
> sbt, have you been running the test suite before submitting patches?  
> If not, then please do.

I ran it after I submitted.  Sorry.

Here is another patch.  It also makes sure that __self__ is reported as None when METH_STATIC.
msg149981 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-12-21 10:35
Thanks for the patch. Since you modified PyCFunction_GET_SELF to return NULL for static methods, why not simply use it instead of manually looking up the flags in several places?

I'm talking about the following changes:

-    PyObject *self = PyCFunction_GET_SELF(func);
+    int flags = PyCFunction_GET_FLAGS(func);
+    PyObject *self = flags & METH_STATIC ? NULL :
+        _PyCFunction_GET_RAW_SELF(func);

[...]

     self = m->m_self;
-    if (self == NULL)
+    if (self == NULL || PyCFunction_GET_FLAGS(m) & METH_STATIC)
         self = Py_None;

[...]

-            PyObject *self = PyCFunction_GET_SELF(func);
+            PyObject *self = flags & METH_STATIC ? NULL :
+                _PyCFunction_GET_RAW_SELF(func);


Unless you demonstrate there's a significant performance improvement in this style, we should really favour the simpler style.
msg150010 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2011-12-21 16:31
A simplified patch getting rid of _PyCFunction_GET_RAW_SELF().
msg150152 - (view) Author: Roundup Robot (python-dev) Date: 2011-12-23 11:41
New changeset 7a7b698f6f21 by Antoine Pitrou in branch 'default':
Issue #13577: Built-in methods and functions now have a __qualname__.
http://hg.python.org/cpython/rev/7a7b698f6f21
msg150153 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-12-23 11:42
The patch is committed, thank you!
History
Date User Action Args
2011-12-23 11:42:04pitrousetstatus: open -> closed
resolution: rejected
messages: + msg150153

stage: patch review -> resolved
2011-12-23 11:41:07python-devsetmessages: + msg150152
2011-12-21 16:31:43sbtsetfiles: + method_qualname.patch

messages: + msg150010
2011-12-21 10:35:17pitrousetmessages: + msg149981
2011-12-13 13:14:12sbtsetfiles: + method_qualname.patch

messages: + msg149385
2011-12-13 05:57:55meador.ingesetmessages: + msg149379
2011-12-12 14:19:50sbtsetfiles: + method_qualname.patch

messages: + msg149302
2011-12-12 13:27:09pitrousetnosy: + gvanrossum
messages: + msg149299
2011-12-12 12:47:41python-devsetnosy: + python-dev
messages: + msg149296
2011-12-12 11:56:22sbtsetfiles: + descr_qualname.patch

messages: + msg149293
2011-12-12 11:49:55sbtsetfiles: + method_qualname.patch

messages: + msg149291
2011-12-12 11:05:58pitrousetmessages: + msg149286
stage: needs patch -> patch review
2011-12-12 00:03:58sbtsetfiles: + descr_qualname.patch

messages: + msg149262
2011-12-11 19:11:16pitrousetmessages: + msg149240
2011-12-11 19:04:40sbtsetmessages: + msg149238
2011-12-11 18:32:22pitrousetmessages: + msg149235
2011-12-11 16:26:32jceasetnosy: + jcea
2011-12-11 14:33:52sbtsetfiles: + descr_qualname.patch

messages: + msg149226
2011-12-11 13:30:14sbtsetfiles: + descr_qualname.patch

nosy: + sbt
messages: + msg149225

keywords: + patch
2011-12-11 04:27:04meador.ingecreate