gh-108191: Add support of positional argument in SimpleNamespace constructor#108195
Conversation
…e constructor
SimpleNamespace({'a': 1, 'b': 2}) and SimpleNamespace([('a', 1), ('b', 2)])
are now the same as SimpleNamespace(a=1, b=2).
213c74d to
c761180
Compare
carljm
left a comment
There was a problem hiding this comment.
This looks like a convenient option to provide in the constructor, with no downside, and established precedent in the dict constructor.
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
erlend-aasland
left a comment
There was a problem hiding this comment.
Thanks! The "polish" commit was a nice clean-up; it made the code more readable, IMO.
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
|
I have no real objections. I had considered doing this originally but figured it could wait. Here we are. 😄 |
|
I tried to implement args = (self.__dict__,)
return type(self)(*args, **kwargs)instead of d = {}
d.update(self.__dict__)
d.update(kwargs)
return type(self)(**d) |
|
On other hand, I decided to use other way (#109175), which is more consistent with pickling and copying: result = type(self)()
result.__dict__.update(self.__dict__)
result.__dict__.update(kwargs)
return resultSo this is no longer a blocker. |
|
When you're done making the requested changes, leave the comment: |
Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com> Co-authored-by: Pieter Eendebak <pieter.eendebak@gmail.com>
Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
|
I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @erlend-aasland, @ericsnowcurrently, @AA-Turner, @carljm: please review the changes made to this pull request. |
ericsnowcurrently
left a comment
There was a problem hiding this comment.
LGTM
I left one comment about something mostly unrelated to this PR, so I don't consider it a blocker.
|
It would be good to get @rhettinger's feedback, but this PR already received many positive feedbacks, and I already invested in this issue so much, that I'm going to merge it anyway, even if initially I was not so interested in this. |
carljm
left a comment
There was a problem hiding this comment.
Seems to be a reasonable amount of interest in this, and plenty of review approvals. Can we go ahead and merge for 3.13?
|
Looks like a merge conflict needs to be resolved. |
SimpleNamespace({'a': 1, 'b': 2}) and SimpleNamespace([('a', 1), ('b', 2)]) are now the same as SimpleNamespace(a=1, b=2).
📚 Documentation preview 📚: https://cpython-previews--108195.org.readthedocs.build/