| msg149209 - (view) |
Author: Meador Inge (meador.inge) *  |
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 "", line 1, in AttributeError: 'builtin_function_or_method' object has no attribute '__qualname__' >>> str.strip.__qualname__ Traceback (most recent call last): File "", line 1, in AttributeError: 'method_descriptor' object has no attribute '__qualname__' I will work up a patch. |
|
|
| msg149225 - (view) |
Author: Richard Oudkerk (sbt) * |
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) * |
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) * |
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) * |
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) * |
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) * |
Date: 2011-12-12 00:03 |
| New version of the patch with tests and using _Py_IDENTIFIER. |
|
|
| msg149286 - (view) |
Author: Antoine Pitrou (pitrou) * |
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) * |
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) * |
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) * |
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 "", line 1, in AttributeError: 'method_descriptor' object has no attribute '__self__' >>> list().append.__self__ [] |
|
|
| msg149302 - (view) |
Author: Richard Oudkerk (sbt) * |
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) * |
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) * |
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) * |
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) * |
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) * |
Date: 2011-12-23 11:42 |
| The patch is committed, thank you! |
|
|