Skip to content

bpo-42344: Improve pseudo implementation for SimpleNamespace#23264

Merged
ericvsmith merged 4 commits into
python:masterfrom
jugmac00:update-simplenamespace-example
Nov 13, 2020
Merged

bpo-42344: Improve pseudo implementation for SimpleNamespace#23264
ericvsmith merged 4 commits into
python:masterfrom
jugmac00:update-simplenamespace-example

Conversation

@jugmac00

@jugmac00 jugmac00 commented Nov 13, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

Wheras SimpleNamespace is implemented in C, the documentation shows a
pseudo implementation in Python for easier understanding.

The magic method for the comparison did not take into account that the
actual C implementation also compares the types.

__eq__ of the Python pseudo implementation now also compares the types.

https://bugs.python.org/issue42344

@jugmac00
bpo-38914: Improve pseudo implementation for SimpleNamespace
669a207 
Wheras `SimpleNamespace` is implemented in C, the documentation shows a
pseudo implementation in Python for easier understanding.

The magic method for the comparison did not take into account that the
actual C implementation also compares the types.

`__eq__` of the Python pseudo implementation now also compares the types.
@bedevere-bot bedevere-bot added docs Documentation in the Doc dir awaiting review labels Nov 13, 2020
@ericvsmith

ericvsmith commented Nov 13, 2020

Copy link
Copy Markdown
Member

Shouldn't the bpo be https://bugs.python.org/issue42344?

ericvsmith
ericvsmith reviewed Nov 13, 2020
View reviewed changes
Comment thread Doc/library/types.rst Outdated
@jugmac00 jugmac00 changed the title bpo-38914: Improve pseudo implementation for SimpleNamespace bpo-42344: Improve pseudo implementation for SimpleNamespace Nov 13, 2020
@jugmac00
Apply suggested changes
0c03224 
modified:   library/types.rst
ericvsmith
ericvsmith reviewed Nov 13, 2020
View reviewed changes
Comment thread Doc/library/types.rst Outdated
asvetlov
asvetlov requested changes Nov 13, 2020
View reviewed changes
Comment thread Doc/library/types.rst Outdated
@bedevere-bot

bedevere-bot commented Nov 13, 2020

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@jugmac00
Apply requested changes
c07831d 
modified:   library/types.rst
@ericvsmith

ericvsmith commented Nov 13, 2020

Copy link
Copy Markdown
Member

Shouldn't the method return NotImplemented if the other type mismatches?
Is there a possibility when self is not an instance of SimpleNamespace?

Yes, I noted that above. If you want to be exact, I gave the code above. #23264 (comment)

I guess we should be exactly precise, since the snippet is so short.

ericvsmith
ericvsmith requested changes Nov 13, 2020
View reviewed changes
Comment thread Doc/library/types.rst Outdated
@ericvsmith
ericvsmith merged commit bbeb2d2 into python:master Nov 13, 2020
@ericvsmith

ericvsmith commented Nov 13, 2020

Copy link
Copy Markdown
Member

Thanks for your patience!

@jugmac00
jugmac00 deleted the update-simplenamespace-example branch November 13, 2020 18:16
@miss-islington

miss-islington commented Nov 13, 2020

Copy link
Copy Markdown
Contributor

Thanks @jugmac00 for the PR, and @ericvsmith for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@miss-islington

miss-islington commented Nov 13, 2020

Copy link
Copy Markdown
Contributor

Thanks @jugmac00 for the PR, and @ericvsmith for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 13, 2020
@jugmac00 @miss-islington
bpo-42344: Improve pseudo implementation for SimpleNamespace (pythonG…
f411cee 
…H-23264)

(cherry picked from commit bbeb2d2)

Co-authored-by: Jürgen Gmach <juergen.gmach@googlemail.com>
@bedevere-bot

bedevere-bot commented Nov 13, 2020

Copy link
Copy Markdown

GH-23269 is a backport of this pull request to the 3.8 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 13, 2020
@jugmac00 @miss-islington
bpo-42344: Improve pseudo implementation for SimpleNamespace (pythonG…
bd525e5 
…H-23264)

(cherry picked from commit bbeb2d2)

Co-authored-by: Jürgen Gmach <juergen.gmach@googlemail.com>
@bedevere-bot

bedevere-bot commented Nov 13, 2020

Copy link
Copy Markdown

GH-23270 is a backport of this pull request to the 3.9 branch.

ericvsmith pushed a commit that referenced this pull request Nov 13, 2020
@miss-islington @jugmac00
bpo-42344: Improve pseudo implementation for SimpleNamespace (GH-23264)…
cb2b203 
… (GH-23269)

(cherry picked from commit bbeb2d2)

Co-authored-by: Jürgen Gmach <juergen.gmach@googlemail.com>

Co-authored-by: Jürgen Gmach <juergen.gmach@googlemail.com>
ericvsmith pushed a commit that referenced this pull request Nov 13, 2020
@miss-islington @jugmac00
bpo-42344: Improve pseudo implementation for SimpleNamespace (GH-23264)…
4defeb0 
… (GH-23270)

(cherry picked from commit bbeb2d2)

Co-authored-by: Jürgen Gmach <juergen.gmach@googlemail.com>

Co-authored-by: Jürgen Gmach <juergen.gmach@googlemail.com>
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
@jugmac00 @adorilson
bpo-42344: Improve pseudo implementation for SimpleNamespace (pythonG…
f1a4d99 
…H-23264)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ericvsmith ericvsmith ericvsmith approved these changes
@asvetlov asvetlov Awaiting requested review from asvetlov

Assignees

No one assigned

Labels

docs Documentation in the Doc dir skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@jugmac00 @ericvsmith @bedevere-bot @miss-islington @asvetlov @the-knights-who-say-ni