Use types.SimpleNamespace for argparse.Namespace

[ericsnowcurrently]

BPO	39076
Nosy	@rhettinger, @ericsnowcurrently

^{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:

assignee = 'https://github.com/rhettinger'
closed_at = <Date 2020-05-18.02:04:09.157>
created_at = <Date 2019-12-17.16:35:51.712>
labels = ['library', '3.9']
title = 'Use types.SimpleNamespace for argparse.Namespace'
updated_at = <Date 2020-05-18.02:04:09.157>
user = 'https://github.com/ericsnowcurrently'

bugs.python.org fields:

activity = <Date 2020-05-18.02:04:09.157>
actor = 'rhettinger'
assignee = 'rhettinger'
closed = True
closed_date = <Date 2020-05-18.02:04:09.157>
closer = 'rhettinger'
components = ['Library (Lib)']
creation = <Date 2019-12-17.16:35:51.712>
creator = 'eric.snow'
dependencies = []
files = []
hgrepos = []
issue_num = 39076
keywords = []
message_count = 4.0
messages = ['358555', '358611', '358911', '358912']
nosy_count = 3.0
nosy_names = ['rhettinger', 'eric.snow', 'paul.j3']
pr_nums = []
priority = 'normal'
resolution = 'rejected'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue39076'
versions = ['Python 3.9']

[ericsnowcurrently]

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.

[rhettinger]

There are some sticking points:

• types.SimpleNamespace() sorts attributes, so this would get in the way of issue bpo-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

[ericsnowcurrently]

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 bpo-39058.

hence bpo-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.

[ericsnowcurrently]

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.