Improve repr for structseq objects to show named, but unindexed fields

[rhettinger]

BPO	11698
Nosy	@rhettinger, @pitrou, @ezio-melotti, @merwok, @ericsnowcurrently, @lilydjwg, @berkerpeksag, @serhiy-storchaka, @phmc, @mblahay
Files	issue11698.patch structseq_2.patch: Show named but unindexed fields under the dict keyword argument

^{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 = 'https://github.com/pitrou' closed_at = None created_at = labels = ['interpreter-core', 'easy', 'type-feature', '3.8'] title = 'Improve repr for structseq objects to show named, but unindexed fields' updated_at = user = 'https://github.com/rhettinger' ``` bugs.python.org fields: ```python activity = actor = 'serhiy.storchaka' assignee = 'pitrou' closed = False closed_date = None closer = None components = ['Interpreter Core'] creation = creator = 'rhettinger' dependencies = [] files = ['26391', '32527'] hgrepos = [] issue_num = 11698 keywords = ['patch', 'easy'] message_count = 14.0 messages = ['132372', '153754', '153755', '155261', '165570', '188416', '200624', '200718', '202332', '202364', '224473', '341578', '341591', '378933'] nosy_count = 13.0 nosy_names = ['rhettinger', 'pitrou', 'ezio.melotti', 'eric.araujo', 'Arfrever', 'eric.snow', 'lilydjwg', 'berker.peksag', 'dlam', 'serhiy.storchaka', 'pconnell', 'sunfinite', 'mblahay'] pr_nums = [] priority = 'low' resolution = None stage = 'patch review' status = 'open' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue11698' versions = ['Python 3.8'] ```

[rhettinger]

The current __repr__ for structseq only shows the name/value pairs for the positional part and it ignores the other named fields.

For example, os.stat(somefile) returns: posix.stat_result(st_mode=33277, st_ino=8468407, st_dev=234881026, st_nlink=1, st_uid=0, st_gid=80, st_size=25424, st_atime=1301263901, st_mtime=1298229258, st_ctime=1298283922)

but it doesn't show the other named fields and their values: {'st_ctime': 1298283922.0, 'st_rdev': 0, 'st_mtime': 1298229258.0, 'st_blocks': 56, 'st_flags': 0, 'st_gen': 0, 'st_atime': 1301263901.0, 'st_blksize': 4096, 'st_birthtime': 1298229258.0}

The __reduce__ method for structseq returns both the tuple portion and the dictionary portion. The latter needs to be added to the repr so that information doesn't get hidden from the user.

[berkerpeksag]

I'd like to work on this issue. I found the Objects/structseq.c [1] file. Am I on the right path?

Thanks!

[1] http://hg.python.org/cpython/file/5b4b70bd2b6f/Objects/structseq.c#l157

[merwok]

You are! See the devguide for more setup guidelines.

[pitrou]

+1. Also, the repr() should show the float values of st_mtime and friends, rather than truncated integers.

[4fddca73-bb6c-4257-9ca8-057376e53120]

hi hi, found this bug after clicking the "Easy issues" link

i basically just took Ray's hint to look at the __reduce method, and applied it to the __repr method in this patch

also updated is the test_repr() unittest

[pitrou]

Thanks for the patch. (Yes, I'm looking at this a bit late :-))

First, there seems to be a problem with the repr() of of.stat() results:

./python -c "import os; print(os.stat('LICENSE'))" posix.stat_result(st_mode=33204, st_ino=6553619, st_dev=2053, st_nlink=1, st_uid=1000, st_gid=1000, st_size=15089, st_atime=1367693898, st_mtime=1365264866, st_ctime=1366481591, st_atime=1367693898.528636, st_mtime=1365264866.4163036, st_ctime=1366481591.9862735, st_atime_ns=1367693898528635928, st_mtime_ns=1365264866416303676, st_ctime_ns=1366481591986273627, st_blksize=4096, st_blocks=32, st_rdev=0)

As you see, fields such as "st_atime" are duplicated.

There are other issues with the patch:

• C variable declarations should always be at the beginning of blocks (otherwise it's not C89-compliant)

• C++-style comments (//) are forbidden

• I don't understand in which circumstances Py_TYPE(obj)->tp_members[i-n_unnamed_fields].name can be NULL

[09320ff4-bc48-4cea-b5b3-47c4956288e3]

Added patch for 3.4.

The patch demarcates the output by adding a {...} around the dictionary portion. Please let me know if this is the right format or if not required at all. It is a simple change.

[pitrou]

Hmm, does anyone have an opinion for or against the proposed representation in Sunny's patch?

Sunny, if you haven't done so, could you sign a contributor's agreement? http://www.python.org/psf/contrib/ Thanks!

[09320ff4-bc48-4cea-b5b3-47c4956288e3]

The previous patch had a wrong mapping between keys and values. The current implementation of repr means that duplicated keys will be present when invisible fields are included. See points 2 and 3 in http://bugs.python.org/issue1820#msg202330 for more explanation.

I have sidestepped that issue by placing invisible fields under the dict argument. This also plays well with the current code in structseq_new and eval(repr(obj)) works.

The output with the patch is:

$./python -c "import os; print(os.stat('LICENSE'))" os.stat_result(st_mode=33188, st_ino=577299, st_dev=64512, st_nlink=1, st_uid=33616, st_gid=600, st_size=12749, st_atime=1382696747, st_mtime=1382361968, st_ctime=1382361968, dict={'st_atime':1382696747.0, 'st_mtime':1382361968.0, 'st_ctime':1382361968.0, 'st_atime_ns':1382696747000000000, 'st_mtime_ns':1382361968000000000, 'st_ctime_ns':1382361968000000000, 'st_blksize':4096, 'st_blocks':32, 'st_rdev':0})

[80036ac5-bb84-4d39-8416-02cd8e51707d]

IMHO '*' could be used as a separator, since relation between indexable fields and named, unindexable fields is similar to relation between positional-or-keyword parameters and keyword-only parameters.

$./python -c "import os; print(os.stat('LICENSE'))" os.stat_result(st_mode=33188, st_ino=577299, st_dev=64512, st_nlink=1, st_uid=33616, st_gid=600, st_size=12749, st_atime=1382696747, st_mtime=1382361968, st_ctime=1382361968, *, st_atime=1382696747.0, st_mtime=1382361968.0, st_ctime=1382361968.0, st_atime_ns=1382696747000000000, st_mtime_ns=1382361968000000000, st_ctime_ns=1382361968000000000, st_blksize=4096, st_blocks=32, st_rdev=0)

[83d2e70e-e599-4a04-b820-3814bbdb9bef]

Could somebody pick this up please as it fixes bpo-5907.

[46c9edd5-eba5-4526-bb2f-20744381119f]

I will work on this

[46c9edd5-eba5-4526-bb2f-20744381119f]

I have been advised to avoid enhancements like this one, so I am setting this back down. Also, this should be relabeled as easy(c).

[serhiy-storchaka]

Other problem is that the repr looks like an evaluable expression, but evaluating it will always produce error.

>>> st = os.stat('/dev/null')
>>> st
os.stat_result(st_mode=8630, st_ino=6, st_dev=6, st_nlink=1, st_uid=0, st_gid=0, st_size=0, st_atime=1602523313, st_mtime=1602523313, st_ctime=1602523313)
>>> os.stat_result(st_mode=8630, st_ino=6, st_dev=6, st_nlink=1, st_uid=0, st_gid=0, st_size=0, st_atime=1602523313, st_mtime=1602523313, st_ctime=1602523313)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: structseq() takes at most 2 keyword arguments (10 given)

os.stat_result() accepts only two arguments: a tuple for indexable elements and a dict for non-indexable elements.

>>> os.stat_result((8630, 6, 6, 1, 0, 0, 0, 1602523313, 1602523313, 1602523313), {'st_atime': 1602523313.282834, 'st_mtime': 1602523313.282834, 'st_ctime': 1602523313.282834, 'st_atime_ns': 1602523313282834115, 'st_mtime_ns': 1602523313282834115, 'st_ctime_ns': 1602523313282834115, 'st_blksize': 4096, 'st_blocks': 0, 'st_rdev': 259})
os.stat_result(st_mode=8630, st_ino=6, st_dev=6, st_nlink=1, st_uid=0, st_gid=0, st_size=0, st_atime=1602523313, st_mtime=1602523313, st_ctime=1602523313)

But such form looks not very readable, because it lacks names for indexable elements.

To solve this we can use an angular form in the repr:

\<os.stat_result ...>