Skip to content
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

gh-112125: Fix None.__ne__(None) returning NotImplemented instead of False #112504

Merged
merged 1 commit into from
Dec 7, 2023
Merged

gh-112125: Fix None.__ne__(None) returning NotImplemented instead of False #112504

merged 1 commit into from
Dec 7, 2023

Conversation

andrewluotechnologies
Copy link
Contributor

@andrewluotechnologies andrewluotechnologies commented Nov 28, 2023 •
edited
Loading

I tested this fix on Windows 11 with VS2022. Compiled and ran None.__ne__(None) before and after my changes. Result before: NotImplemented, after: False

Note: this is my first pull request/contributing and this code wasn't my original idea - thanks to @rhettinger for suggesting the route to fix this. I only implemented the code.

@cpython-cla-bot CPython CLA Bot
Copy link

cpython-cla-bot bot commented Nov 28, 2023 •
edited
Loading

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app bedevere-app bot mentioned this pull request Nov 28, 2023
Closed
@bedevere-app
Copy link

bedevere-app bot commented Nov 28, 2023

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

Eclips4
Eclips4 reviewed Nov 30, 2023
View reviewed changes
Objects/object.c Outdated Show resolved Hide resolved
@bedevere-app
Copy link

bedevere-app bot commented Dec 3, 2023

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@bedevere-app
Copy link

bedevere-app bot commented Dec 3, 2023

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

vstinner
vstinner reviewed Dec 5, 2023
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind to add a test and add a Changelog (NEWS) entry? See https://devguide.python.org/ on how to add a Changelog entry.

@andrewluotechnologies andrewluotechnologies force-pushed the andrew branch 3 times, most recently from 237be2c to 1bb1748 Compare December 6, 2023 13:28
vstinner
vstinner reviewed Dec 6, 2023
View reviewed changes
Lib/test/test_builtin.py Show resolved Hide resolved
Objects/typeobject.c Outdated Show resolved Hide resolved
@Yhg1s Yhg1s added the needs backport to 3.12 bug and security fixes label Dec 7, 2023
@vstinner vstinner merged commit 9c3458e into python:main Dec 7, 2023
28 checks passed
@miss-islington-app
Copy link

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

@miss-islington-app
Copy link

Sorry, @andrewluotechnologies and @vstinner, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 9c3458e05865093dd55d7608810a9d0ef0765978 3.12

vstinner pushed a commit to vstinner/cpython that referenced this pull request Dec 7, 2023
@andrewluotechnologies @vstinner
pythongh-112125: Fix None.__ne__(None) returning NotImplemented inste…
1a07240 
…ad of False (python#112504)

(cherry picked from commit 9c3458e)
@vstinner vstinner mentioned this pull request Dec 7, 2023
Merged
@bedevere-app
Copy link

bedevere-app bot commented Dec 7, 2023

GH-112827 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Dec 7, 2023
@vstinner
Copy link
Member

vstinner commented Dec 7, 2023

Merged, thank you. I backported the change manually to 3.12 to fix a merge conflict.

vstinner added a commit that referenced this pull request Dec 7, 2023
@vstinner @andrewluotechnologies
[3.12] gh-112125: Fix None.__ne__(None) returning NotImplemented inst…
f272716 
…ead of … (#112827)

gh-112125: Fix None.__ne__(None) returning NotImplemented instead of False (#112504)

(cherry picked from commit 9c3458e)

Co-authored-by: andrewluotechnologies <44252973+andrewluotechnologies@users.noreply.github.com>
@andrewluotechnologies andrewluotechnologies deleted the andrew branch December 7, 2023 15:40
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
@andrewluotechnologies @aisk
pythongh-112125: Fix None.__ne__(None) returning NotImplemented inste…
93d6e49 
…ad of False (python#112504)
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@andrewluotechnologies @Glyphack
pythongh-112125: Fix None.__ne__(None) returning NotImplemented inste…
cfac9d4 
…ad of False (python#112504)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

@rhettinger rhettinger Awaiting requested review from rhettinger

@Eclips4 Eclips4 Awaiting requested review from Eclips4

@vstinner vstinner Awaiting requested review from vstinner

Assignees

@vstinner vstinner

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@andrewluotechnologies @vstinner @Eclips4 @Yhg1s