8000 bpo-37579: Return NotImplemented in Python implementation of __eq__ to match C implementation of datetime for timedelta and time by tirkarthi · Pull Request #14726 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

bpo-37579: Return NotImplemented in Python implementation of __eq__ to match C implementation of datetime for timedelta and time #14726

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 3 commits into from
Jul 13, 2019
Merged

bpo-37579: Return NotImplemented in Python implementation of __eq__ to match C implementation of datetime for timedelta and time #14726

merged 3 commits into from
Jul 13, 2019

Conversation

tirkarthi
Copy link
Member
@tirkarthi tirkarthi commented Jul 13, 2019 •
edited by bedevere-bot
Loading

When the other object being compared is not the same type then :exc:NotImplemented is returned in C implementation of datetime.timedelta and datetime.time. Change the Python implementation to match the C implementation

https://bugs.python.org/issue37579

tirkarthi added 2 commits July 13, 2019 10:56
@tirkarthi
Return NotImplemented for timedelta and time in __eq__ for different …
622909e 
…types in Python implementation to match C implementation
@tirkarthi
Add NEWS
55e2c39
jdemeyer
jdemeyer approved these changes Jul 13, 2019
View reviewed changes
Copy link
Contributor
@jdemeyer jdemeyer left a comment

Choose a reason for hiding this comment

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

Code makes sense, test added.

pganssle
pganssle requested changes Jul 13, 2019
View reviewed changes
Copy link
Member
@pganssle pganssle left a comment

Choose a reason for hiding this comment

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

Thank you for doing this @tirkarthi. I have a few minor quibbles inline, but overall this is a very solid PR.

@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.

@tirkarthi
Copy link
Member Author

@pganssle Thank you for the review. I am convinced to use a dedicated test object instead of mock.ANY. I copied the test class from dateutil. I reworded the code comment along with a test for __ne__. I have made the requested changes; please review again

@bedevere-bot
Copy link

Thanks for making the requested changes!

@pganssle: please review the changes made to this pull request.

@pganssle pganssle merged commit e6b46aa into python:master Jul 13, 2019
@pganssle
Copy link
Member

Thanks @tirkarthi for the PR and @serhiy-storchaka for the review and report.

@tirkarthi
Copy link
Member Author

Thanks Paul. Is this worth backporting? this seems like a bug fix to me.

@serhiy-storchaka
Copy link
Member

This seems like a bug fix to me too.

@pganssle
Copy link
Member

I'm fine with backporting it since it's a PEP 399 violation.

@miss-islington
Copy link
Contributor

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

@miss-islington
Copy link
Contributor

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

@miss-islington
Copy link
Contributor

Sorry @tirkarthi and @pganssle, I had trouble checking out the 3.7 backport branch.
Please backport using cherry_picker on command line.
cherry_picker e6b46aafad3427463d6264a68824df4797e682f1 3.7

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 13, 2019
@tirkarthi @miss-islington
bpo-37579: Improve equality behavior for pure Python datetime and time (
d07eda2 
pythonGH-14726)

Returns NotImplemented for timedelta and time in __eq__ for different types in Python implementation, which matches the C implementation.

This also adds tests to enforce that these objects will fall back to the right hand side's __eq__ and/or __ne__ implementation.

bpo-37579
(cherry picked from commit e6b46aa)

Co-authored-by: Xtreak <tir.karthi@gmail.com>
@bedevere-bot
Copy link

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

tirkarthi added a commit to tirkarthi/cpython that referenced this pull request Jul 13, 2019
@tirkarthi
[3.7] bpo-37579: Improve equality behavior for pure Python datetime a…
f72da05 
…nd time (pythonGH-14726)

Returns NotImplemented for timedelta and time in __eq__ for different types in Python implementation, which matches the C implementation.

This also adds tests to enforce that these objects will fall back to the right hand side's __eq__ and/or __ne__ implementation.

bpo-37579
(cherry picked from commit e6b46aa)

Co-authored-by: Xtreak <tir.karthi@gmail.com>
@bedevere-bot
Copy link

GH-14745 is a backport of this pull request to the 3.7 branch.
8000

miss-islington added a commit that referenced this pull request Jul 13, 2019
@miss-islington @tirkarthi
bpo-37579: Improve equality behavior for pure Python datetime and time (
143672c 
GH-14726)

Returns NotImplemented for timedelta and time in __eq__ for different types in Python implementation, which matches the C implementation.

This also adds tests to enforce that these objects will fall back to the right hand side's __eq__ and/or __ne__ implementation.

bpo-37579
(cherry picked from commit e6b46aa)

Co-authored-by: Xtreak <tir.karthi@gmail.com>
miss-islington pushed a commit that referenced this pull request Jul 14, 2019
@tirkarthi @miss-islington
[3.7] bpo-37579: Improve equality behavior for pure Python datetime a…
c6b3106 
…nd time (GH-14726) (GH-14745)

Returns NotImplemented for timedelta and time in __eq__ for different types in Python implementation, which matches the C implementation.

This also adds tests to enforce that these objects will fall back to the right hand side's __eq__ and/or __ne__ implementation.

[bpo-37579](https://bugs.python.org/issue37579)
(cherry picked from commit e6b46aa)

Co-authored-by: Xtreak <tir.karthi@gmail.com>





https://bugs.python.org/issue37579
@pganssle pganssle mentioned this pull request Jul 28, 2019
Merged
lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
@tirkarthi @lisroach
bpo-37579: Improve equality behavior for pure Python datetime and time (
3046fa2 
pythonGH-14726)

Returns NotImplemented for timedelta and time in __eq__ for different types in Python implementation, which matches the C implementation.

This also adds tests to enforce that these objects will fall back to the right hand side's __eq__ and/or __ne__ implementation.

bpo-37579
@mattip mattip mentioned this pull request Oct 20, 2019
Merged
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 6D40 2020
@tirkarthi @DinoV
bpo-37579: Improve equality behavior for pure Python datetime and time (
ea7d89f 
pythonGH-14726)

Returns NotImplemented for timedelta and time in __eq__ for different types in Python implementation, which matches the C implementation.

This also adds tests to enforce that these objects will fall back to the right hand side's __eq__ and/or __ne__ implementation.

bpo-37579
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pganssle pganssle pganssle approved these changes

@jdemeyer jdemeyer jdemeyer approved these changes

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

@abalkin abalkin Awaiting requested review from abalkin

Assignees

@pganssle pganssle

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@tirkarthi @bedevere-bot @pganssle @serhiy-storchaka @miss-islington @jdemeyer @the-knights-who-say-ni
0