classification
Title: Use types.SimpleNamespace for argparse.Namespace
Type: Stage: resolved
Components: Library (Lib) Versions: Python 3.9
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: eric.snow, paul.j3, rhettinger
Priority: normal Keywords:

Created on 2019-12-17 16:35 by eric.snow, last changed 2020-05-18 02:04 by rhettinger. This issue is now closed.

Messages (4)
msg358555 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2019-12-17 16:35
types.SimpleNamespace does pretty much exactly the same thing as argparse.Namespace.  We should have the latter subclass the former.  I expect the only reason that wasn't done before is because SimpleNamespace is newer.

The only thing argparse.Namespace does differently is it supports the contains() builtin.  So the subclass would look like this:

class Namespace(types.SimpleNamespace):
    """..."""
    def __contains__(self, key):
        return key in self.__dict__

Alternately, we could add __contains__() to SimpleNamespace and then the subclass would effectively have an empty body.
msg358611 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2019-12-18 05:42
There are some sticking points:

* types.SimpleNamespace() sorts attributes, so this would get in the way of issue #39058.

* argparse.Namespace() supports a __contains__() method that isn't offered by types.SimpleNamespace():

    >>> 'abc' in Namespace(abc=10)
    True

* Argparse is sensitive to start-up time so we mostly want to avoid adding new dependencies.  Start-up time recently got worse when the re module added a number of dependencies.

* The __repr__ for SimpleNamespace() doesn't round-trip and isn't what we have now with Namespace.  That potentially breaks doctests and diverges from published examples:

    >>> Namespace(abc=10)
    Namespace(abc=10)
    >>> SimpleNamespace(abc=10)
    namespace(abc=10)

* Ironically, the class name "Namespace" is simpler than "SimpleNamespace" ;-)

* Much of the code in argparse.Namespace() inherits from _AttributeHolder, so switching to types.SimpleNamespace() doesn't really save us much code.

Are there any upsides to switching?  Attribute lookup is almost equally fast using either approach, so there is no speed benefit:

    $ python3.8 -m timeit -r 11 -s 'from argparse import Namespace as NS' -s 'args=NS(abc=10)' 'args.abc'
    10000000 loops, best of 11: 32.7 nsec per loop

    $ python3.8 -m timeit -r 11 -s 'from types import SimpleNamespace as NS' -s 'args=NS(abc=10)' 'args.abc'
    10000000 loops, best of 11: 32.4 nsec per loop
msg358911 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2019-12-27 20:06
Sorry if there was any confusion.  I didn't mean to suggest we get rid
of argparse.Namespace (in favor of SimpleNamespace).  Rather, the
former would subclass the latter.

> * types.SimpleNamespace() sorts attributes, so this would get in the way of issue #39058.

hence issue 39075

> * argparse.Namespace() supports a __contains__() method that isn't offered by types.SimpleNamespace():

As I suggested originally, there isn't any problem here if
argparse.Namespace subclasses SimpleNamespace.

> * Argparse is sensitive to start-up time so we mostly want to avoid adding new dependencies.

I agree on avoiding extra imports.  The simple solution right now is
to use "type(sys.implementation)", though that isn't the clearest
code.

FWIW, this would also be solved if we added SimpleNamespace to the
builtins, but that is a separate discussion. :)

> * The __repr__ for SimpleNamespace() doesn't round-trip and isn't what we have now with Namespace.

We could certainly fix the repr, but that's kind of irrelevant here if
argparse.Namespace is a subclass.

FWIW, this is also resolved if we add SimpleNamespace to the builtins
(as "namespace").

> * Ironically, the class name "Namespace" is simpler than "SimpleNamespace" ;-)

Agreed. :)  We only used that long name because putting it in the
"types" module mean it needed to have an extra clear name.

That said, it is irrelevant here if argparse.Namespace is a subclass

> * Much of the code in argparse.Namespace() inherits from _AttributeHolder, so switching to types.SimpleNamespace() doesn't really save us much code.

Honestly, _AttributeHolder isn't needed.  The only thing it provides
is a nice repr (like SimpleNamespace does), with some extra
functionality for dynamically sorting/filtering down the attrs shown
in the repr.   There are 3 subclasses and Namespace doesn't even use
the attr filtering.  The implementation doesn't seem to warrant a
dedicated base class and it would be simpler for those 2 subclasses to
each to have a dedicated __repr__ implementation (and for Namespace to
subclass SimpleNamespce), rather than to subclass _AttributeHolder.
Subclassing from _AttributeHolder gives the illusion that there is
something more going on there than there actually is.

Aside from that, there's the weaker argument about consistency and
avoiding duplication (i.e. SimpleNamespace is the canonical "simple"
namespace implementation in Python).  Also, if SimpleNamespace had
existed when argparse was written then I expect it would have been
used instead.

> Are there any upsides to switching?  Attribute lookup is almost equally fast using either approach, so there is no speed benefit:

Yeah, I didn't expect there to much difference in performance.
msg358912 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2019-12-27 20:07
Anyway, this probably isn't a discussion worth extending much further.  I don't think it's important enough. :)  So if you have reservations about this then feel free to close the issue.
History
Date User Action Args
2020-05-18 02:04:09rhettingersetstatus: open -> closed
resolution: rejected
stage: test needed -> resolved
2019-12-27 20:07:27eric.snowsetmessages: + msg358912
2019-12-27 20:06:13eric.snowsetmessages: + msg358911
2019-12-18 05:42:15rhettingersetassignee: rhettinger
messages: + msg358611
2019-12-17 21:37:13paul.j3setnosy: + paul.j3
2019-12-17 16:42:13xtreaksetnosy: + rhettinger
2019-12-17 16:35:51eric.snowcreate