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-121039: add Floats/ComplexesAreIdenticalMixin to test.support.testcase #121071

Merged
merged 7 commits into from
Sep 8, 2024
Merged

gh-121039: add Floats/ComplexesAreIdenticalMixin to test.support.testcase #121071

merged 7 commits into from
Sep 8, 2024

Conversation

skirpichev
Copy link
Member

@skirpichev skirpichev commented Jun 27, 2024 •
edited by bedevere-app bot
Loading

@bedevere-app bedevere-app bot mentioned this pull request Jun 27, 2024
Open
@skirpichev

This comment was marked as outdated.

Sign in to view
@skirpichev skirpichev mentioned this pull request Jun 27, 2024
Merged
sobolevn
sobolevn approved these changes Jun 27, 2024
View reviewed changes
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Seems like a good refactoring, thanks!

@sobolevn
Copy link
Member

Build failure is not related and is reported here #121065

@skirpichev
Copy link
Member Author

There are more cases, which could be changed, some were mentioned in the issue. But I'm not sure it worth - stopping reinvent the wheel in new tests seems enough.

@skirpichev
Copy link
Member Author

Unfortunately, #120894 was merged first. Yet one place to refactor, fixed.

@skirpichev skirpichev mentioned this pull request Jul 12, 2024
Open
@skirpichev skirpichev mentioned this pull request Sep 8, 2024
Open
serhiy-storchaka
serhiy-storchaka approved these changes Sep 8, 2024
View reviewed changes
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM.

@serhiy-storchaka
Copy link
Member

I think that test_capi/test_getargs.py is a fair case, and pretty simple too.

Cases in test_math.py are more complex.

@skirpichev
Copy link
Member Author

I think that test_capi/test_getargs.py is a fair case, and pretty simple too.

Yeah, added this.

Cases in test_math.py are more complex.

Some seems mechanical too (assertEqualSign helper). But I'm not sure if this worth it.

@serhiy-storchaka serhiy-storchaka merged commit 8ef8354 into python:main Sep 8, 2024
32 checks passed
@serhiy-storchaka serhiy-storchaka added needs backport to 3.12 bug and security fixes needs backport to 3.13 bugs and security fixes labels Sep 8, 2024
@miss-islington-app
Copy link

Thanks @skirpichev for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@miss-islington-app
Copy link

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

@miss-islington-app
Copy link

Sorry, @skirpichev and @serhiy-storchaka, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 8ef8354ef15e00d484ac2ded9442b789c24b11e0 3.13

@miss-islington-app
Copy link

Sorry, @skirpichev and @serhiy-storchaka, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 8ef8354ef15e00d484ac2ded9442b789c24b11e0 3.12

@serhiy-storchaka serhiy-storchaka added the tests Tests in the Lib/test dir label Sep 8, 2024
@skirpichev skirpichev deleted the num-areidentical-mixin-121039 branch September 8, 2024 13:21
@skirpichev
Copy link
Member Author

I'm working on backports.

skirpichev added a commit to skirpichev/cpython that referenced this pull request Sep 8, 2024
@skirpichev
redo on top of python#121071
42c3783
skirpichev added a commit to skirpichev/cpython that referenced this pull request Sep 8, 2024
@skirpichev
[3.13] pythongh-121039: add Floats/ComplexesAreIdenticalMixin to test…
5fe5b0e 
….support.testcase (pythonGH-121071)

(cherry picked from commit 8ef8354)

Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Sep 8, 2024

GH-123840 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Sep 8, 2024
@bedevere-app
Copy link

bedevere-app bot commented Sep 8, 2024

GH-123841 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 Sep 8, 2024
vstinner pushed a commit that referenced this pull request Sep 11, 2024
@skirpichev
[3.12] gh-121039: add Floats/ComplexesAreIdenticalMixin to test.suppo…
ec119dc 
…rt.testcase (GH-121071) (#123841)

* [3.12] gh-121039: add Floats/ComplexesAreIdenticalMixin to test.support.testcase (GH-121071)
(cherry picked from commit 8ef8354)

Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

@sobolevn sobolevn sobolevn approved these changes

Assignees

@serhiy-storchaka serhiy-storchaka

Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@skirpichev @sobolevn @serhiy-storchaka