Issue39076
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2019-12-17 16:35 by eric.snow, last changed 2022-04-11 14:59 by admin. This issue is now closed.
Messages (4) | |||
---|---|---|---|
msg358555 - (view) | Author: Eric Snow (eric.snow) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
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 |
2022-04-11 14:59:24 | admin | set | github: 83257 |
2020-05-18 02:04:09 | rhettinger | set | status: open -> closed resolution: rejected stage: test needed -> resolved |
2019-12-27 20:07:27 | eric.snow | set | messages: + msg358912 |
2019-12-27 20:06:13 | eric.snow | set | messages: + msg358911 |
2019-12-18 05:42:15 | rhettinger | set | assignee: rhettinger messages: + msg358611 |
2019-12-17 21:37:13 | paul.j3 | set | nosy:
+ paul.j3 |
2019-12-17 16:42:13 | xtreak | set | nosy:
+ rhettinger |
2019-12-17 16:35:51 | eric.snow | create |