8000 bpo-42344: Improve pseudo implementation for SimpleNamespace by jugmac00 · Pull Request #23264 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

bpo-42344: Improve pseudo implementation for SimpleNamespace #23264

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Nov 13, 2020
Merged

bpo-42344: Improve pseudo implementation for SimpleNamespace #23264

merged 4 commits into from
Nov 13, 2020

Conversation

jugmac00
Copy link
Contributor
@jugmac00 jugmac00 commented Nov 13, 2020 •
edited
Loading

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
Copy link
Member

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

ericvsmith
ericvsmith reviewed Nov 13, 2020
View reviewed changes
@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
asvetlov
asvetlov requested changes Nov 13, 2020
View reviewed changes
@bedevere-bot
Copy link

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
Copy link
8000 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
@ericvsmith ericvsmith merged commit bbeb2d2 into python:master Nov 13, 2020
@ericvsmith
Copy link
Member

Thanks for your patience!

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

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

@miss-islington
Copy link
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
@jug 8000 mac00 @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
Copy link

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
Copy link

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

@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Nov 13, 2020
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
0