[WIP] bpo-13224 Change str for classes, functions and modules by merwok · Pull Request #3608 · python/cpython

merwok · 2017-09-16T02:15:16Z

https://bugs.python.org/issue13224

merwok · 2017-09-16T02:21:39Z


    >>> print(defaultdict)              # show our type
-    <class 'test.test_descrtut.defaultdict'>
+    defaultdict


I think these tests looked better with the previous outputs. Revert changes and change inputs instead (>>> defaultdict should show repr unless doctest doesn’t emulate the REPL displayhook)

merwok · 2017-09-16T02:22:30Z


    >>> C.foo(1)
-    classmethod <class 'test.test_descrtut.C'> 1
+    classmethod C 1


We want classmethod test.test_descrtut.C 1

Turns out we don't: qualname is defined to not include the module name, so C is good here.

Alternative: change the code of the foo method to print repr(cls) instead of cls.

merwok · 2017-09-16T02:23:01Z

+    name = PyModule_GetNameObject((PyObject *)m);
+    if (name == NULL) {
+        /* XXX PyUnicode_FromStringAndSize is documented as part of
+           the legacy API   */


Address this

serhiy-storchaka · 2017-09-16T05:45:39Z

    (hashfunc)meth_hash,                        /* tp_hash */
    PyCFunction_Call,                           /* tp_call */
-    0,                                          /* tp_str */
+    (reprfunc)meth_str,                         /* tp_str */


Just use meth_repr.

The point of the change is to have a __str__ that’s different than __repr__ :) But see the ticket, I think I will scale back this PR to classes only.

Oh well, it is Guido who wanted a unified behaviour for modules, classes and functions (since they are things created by syntax (import*, class and def) with a __name__ attribute).

* I think a module’s __name__ depends on its loader, not on the name used in the import statement (import as doesn’t change __name__)

serhiy-storchaka · 2017-09-16T05:51:22Z

+    name = PyModule_GetNameObject((PyObject *)m);
+    if (name == NULL) {
+        /* XXX PyUnicode_FromStringAndSize is documented as part of
+           the legacy API   */


Really? I never heart that PyUnicode_FromStringAndSize is a legacy API. This is the main way of creating strings in C. Could you point the place in the documentation where it declared legacy?

Thanks for the comment. I don’t have the original Mercurial clone on this laptop, so I can’t look for my original commit. This code is removed in my latest changes (not pushed yet).

serhiy-storchaka · 2017-09-16T05:52:54Z

+    if (name == NULL) {
+        /* XXX PyUnicode_FromStringAndSize is documented as part of
+           the legacy API   */
+        PyErr_Clear();


Don't silence all errors. Only AttributeError.

serhiy-storchaka · 2017-09-16T05:53:26Z

+        /* XXX PyUnicode_FromStringAndSize is documented as part of
+           the legacy API   */
+        PyErr_Clear();
+        name = PyUnicode_FromStringAndSize("?", 1);


Wouldn't be better to fall back to repr()?

serhiy-storchaka · 2017-09-16T05:54:19Z

+            return NULL;
+    }
+
+    str = PyUnicode_FromFormat("%U", name);


What is the purpose of this line?

serhiy-storchaka · 2017-09-16T05:57:48Z

+type_str(PyTypeObject *type)
+{
+    PyObject *name, *rtn;
+


Wouldn't be better to include a module name for getting rid of ambiguity?

I would just return PyUnicode_FromString(type->tp_name).

The original discussion on the ticket mentioned only the type qualname, without its module. Looking at Lib/typing.py and other files (for example the error message in functools), I think maybe including the module name too would be useful. Let’s see comments after I get a smaller patch that passes the test suite.

ncoghlan

I think the number of additional changes needed in the test suite and the standard library highlights the compatibility restoration busy-work risks of actually making this change:

while test_structures and test_xmlrpc show the kinds of error messages that the change is aimed at improving (and I think it does indeed improve them), it still requires the kind of test case change that is going to be awkward from projects spanning multiple distinct versions of CPython
most of the other cases were for code that actually wanted the current formatting, and needed to be updated to explicitly request repr() (either by calling it directly, or by switching to !r or %r in a formatting string)

merwok · 2017-09-19T16:09:45Z

Rejected per Nick’s and Guido’s opinions.

Change str for classes, functions and modules

6253875

the-knights-who-say-ni added the CLA signed label Sep 16, 2017

bedevere-bot added the awaiting merge label Sep 16, 2017

add news entry

50c76fa

merwok added awaiting changes and removed awaiting merge labels Sep 16, 2017

merwok commented Sep 16, 2017

View reviewed changes

serhiy-storchaka reviewed Sep 16, 2017

View reviewed changes

merge master

5152334

merwok requested review from ncoghlan and rhettinger as code owners September 19, 2017 03:01

merwok force-pushed the change-function-str-13224 branch from 684c931 to eb99067 Compare September 19, 2017 03:14

fix testsuite

c3c84ad

merwok force-pushed the change-function-str-13224 branch from eb99067 to c3c84ad Compare September 19, 2017 03:35

ncoghlan reviewed Sep 19, 2017

View reviewed changes

merwok closed this Sep 19, 2017

Mariatta removed the awaiting changes label Oct 8, 2017

merwok mentioned this pull request Apr 7, 2025

Change str(x) to return only the qualname for some types #57433

Closed

Uh oh!

Conversation

merwok commented Sep 16, 2017 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

merwok Sep 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ncoghlan left a comment

Choose a reason for hiding this comment

Uh oh!

merwok commented Sep 19, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

merwok commented Sep 16, 2017 •

edited by bedevere-bot

Loading

merwok Sep 19, 2017 •

edited

Loading