GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||
GeezerVison is showing github.com — gh-108191: Add support of positional argument in SimpleNamespace constructor by serhiy-storchaka · Pull Request #108195 · python/cpython Conversation
|
|
I have no real objections. I had considered doing this originally but figured it could wait. Here we are. 😄 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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 result So this is no longer a blocker. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
When you're done making the requested changes, leave the comment: |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Co-authored-by: Eric Snow <[email protected]> Co-authored-by: Pieter Eendebak <[email protected]>
Co-authored-by: Eric Snow <[email protected]>
|
I have made the requested changes; please review again. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks for making the requested changes! @erlend-aasland, @ericsnowcurrently, @AA-Turner, @carljm: please review the changes made to this pull request. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I left one comment about something mostly unrelated to this PR, so I don't consider it a blocker.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Seems to be a reasonable amount of interest in this, and plenty of review approvals. Can we go ahead and merge for 3.13?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Looks like a merge conflict needs to be resolved. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.