classification
Title: improve argparse.Namespace __repr__ for invalid identifiers.
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: berker.peksag Nosy List: berker.peksag, bethard, mbussonn, paul.j3, python-dev, r.david.murray, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2015-06-02 06:00 by mbussonn, last changed 2015-07-29 21:26 by mbussonn. This issue is now closed.

Files
File name Uploaded Description Edit
improve-namespace-repr.patch mbussonn, 2015-06-02 06:00 improve argparse.namespace repr. review
Messages (12)
msg244655 - (view) Author: Matthias Bussonnier (mbussonn) * Date: 2015-06-02 06:00
The argparse Namespace can be missleading in case where the args names are not valid identifiers, eg thinks like a closing bracket:

In [5]: Namespace(a=1, **{')':3})
Out[5]: Namespace()=3, a=1)

more funny:

In [3]: Namespace(a=1, **{s:3})
Out[3]: Namespace(a=1, b=2), Namespace(c=3)

for `s = 'b=2), Namespace(c'`


With this patch the args that are not valid identifiers are shown in ** unpacked-dict, which has the side effect of almost always having repr(eval(repr(obj)))== repr(obj). I'm sure we can find counter example with quotes and triple-quoted string... but anyway.


with this patch (indentation mine for easy comparison):

>>> from argparse import Namespace
>>> Namespace(a=1, **{')': 3})
    Namespace(a=1, **{')': 3})

Which is I think what most user would expect.

Test passes locally (except SSL cert, network thingies, curses and threaded_lru_cache) which look unrelated and is most likely due to my machine.
msg244659 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-06-02 08:36
LGTM.
msg244778 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2015-06-03 17:30
Off hand I don't see a problem with this patch (but I haven't tested it yet).

But I have a couple of cautions:

The docs say, regarding the Namespace class:

> This class is deliberately simple, just an object subclass with a readable string representation.

This patch improves the 'readable' part, but adds some complexity.

The docs also note that the user can provide their own namespace object, and by implication, a custom Namespace class with this improved '__repr__'.

The Namespace '__repr__' is mainly of value during code development, especially when trying ideas in an interactive shell.  It's unlikely that you would want to show the whole namespace to your end user.  So even if your final API requires funny characters, you don't need to use them during development.
msg244779 - (view) Author: Matthias Bussonnier (mbussonn) * Date: 2015-06-03 17:42
Sure and anyway if you have a huge namespace, things will become unreadable. 
But during development/teaching, having object that have a "sane" representation is useful, otherwise your brain (well at least mine), choke on the output and break the flow of my thoughts.

One could also just use __repr(self)__ = repr(self.__dict__), that woudl be even simpler and readable :-)
msg244788 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2015-06-03 20:52
An alternative would be to wrap a non-identifier name in 'repr()':

    def repr1(self):
        def fmt_name(name):
            if name.isidentifier():
                return name
            else:
                return repr(name)
        type_name = type(self).__name__
        arg_strings = []
        for arg in self._get_args():
            arg_strings.append(repr(arg))
        for name, value in self._get_kwargs():
            arg_strings.append('%s=%r' % (fmt_name(name), value))
        return '%s(%s)' % (type_name, ', '.join(arg_strings))

This would produce lines like:

    Namespace(baz='one', 'foo bar'='test', 'x __y'='other')
    
    Namespace(a=1, b=2, 'double " quote'='"', "single ' quote "="'")

    Namespace(')'=3, a=1)

    Namespace(a=1, 'b=2), Namespace(c'=3)

With names that are deliberately messy, it is hard to say which is clearer.
msg244792 - (view) Author: Matthias Bussonnier (mbussonn) * Date: 2015-06-03 23:12
> Namespace(a=1, 'b=2), Namespace(c'=3)

:-) I read that a `prime-b`=2 and `c-prime`=3.

I just feel like having a repr which is closer to the constructor signature is better, but I guess it's a question of taste. Anyway, both would be fine.
msg244795 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2015-06-04 03:21
I mentioned in the related bug/issue that no one has to use odd characters and spaces in the Namespace.  While they are allowed by 'getattr' etc, the programmer has the option of supplying rational names in the 'dest' parameter.  

There's also the question of what kinds of strings can you supply via  'sys.argv'.  For example, I have to use quotes to enter

    $ python echoargv.py '--b=2), Namespace(c' test

Without them 'bash' gives me an error.

Strings like this may be nice for exercising a patch, they may not be realistic in full argparse context.
msg244796 - (view) Author: Matthias Bussonnier (mbussonn) * Date: 2015-06-04 03:57
As far as I remember, argparse is not only use to parse things from sys.argv where the quoting is not necessary. And Namespace is not only use in argparse.

But if you don't want improvement, feel free to close.
msg247580 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-07-29 16:06
If one is going to have a repr at all, I think it should be as accurate as practical.  I think this is worthwhile, and favor the existing patch.
msg247624 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-07-29 20:52
New changeset dcc00d9ba8db by Berker Peksag in branch 'default':
Issue #24360: Improve __repr__ of argparse.Namespace() for invalid identifiers.
https://hg.python.org/cpython/rev/dcc00d9ba8db
msg247626 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2015-07-29 20:53
Thanks for the patch, Matthias.
msg247633 - (view) Author: Matthias Bussonnier (mbussonn) * Date: 2015-07-29 21:26
Thanks for accepting the patch. Looking forward to 3.6 ! :-)
History
Date User Action Args
2015-07-29 21:26:33mbussonnsetmessages: + msg247633
2015-07-29 20:53:28berker.peksagsetstatus: open -> closed
resolution: fixed
messages: + msg247626

stage: commit review -> resolved
2015-07-29 20:52:00python-devsetnosy: + python-dev
messages: + msg247624
2015-07-29 20:03:13berker.peksagsetassignee: berker.peksag
2015-07-29 16:06:09r.david.murraysetnosy: + r.david.murray
messages: + msg247580
2015-06-04 03:57:37mbussonnsetmessages: + msg244796
2015-06-04 03:21:32paul.j3setmessages: + msg244795
2015-06-03 23:12:47mbussonnsetmessages: + msg244792
2015-06-03 20:52:07paul.j3setmessages: + msg244788
2015-06-03 17:42:57mbussonnsetmessages: + msg244779
2015-06-03 17:30:31paul.j3setnosy: + paul.j3
messages: + msg244778
2015-06-02 08:36:58serhiy.storchakasetnosy: + serhiy.storchaka, bethard

messages: + msg244659
stage: patch review -> commit review
2015-06-02 07:23:25berker.peksagsetnosy: + berker.peksag

stage: patch review
2015-06-02 06:00:51mbussonncreate