Skip to content

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

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
serhiy-storchaka merged 7 commits into from
Sep 8, 2024
Merged

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

serhiy-storchaka merged 7 commits into from
Sep 8, 2024

Conversation

@skirpichev
Copy link
Member

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

@bedevere-appbedevere-appbot mentioned this pull request Jun 27, 2024
Closed
@skirpichev

This comment was marked as outdated.

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

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

sobolevn commented Jun 27, 2024

Build failure is not related and is reported here #121065

@skirpichev
Copy link
MemberAuthor

skirpichev commented Jun 27, 2024

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
MemberAuthor

skirpichev commented Jul 10, 2024

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

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

@serhiy-storchakaserhiy-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

serhiy-storchaka commented Sep 8, 2024

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
MemberAuthor

skirpichev commented Sep 8, 2024

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-storchakaserhiy-storchaka merged commit 8ef8354 into python:mainSep 8, 2024
@serhiy-storchakaserhiy-storchaka added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Sep 8, 2024
@miss-islington-app
Copy link

miss-islington-appbot commented Sep 8, 2024

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

miss-islington-appbot commented Sep 8, 2024

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

miss-islington-appbot commented Sep 8, 2024

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

miss-islington-appbot commented Sep 8, 2024

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-storchakaserhiy-storchaka added the tests Tests in the Lib/test dir label Sep 8, 2024
@skirpichevskirpichev deleted the num-areidentical-mixin-121039 branch September 8, 2024 13:21
@skirpichev
Copy link
MemberAuthor

skirpichev commented Sep 8, 2024

I'm working on backports.

skirpichev added a commit to skirpichev/cpython that referenced this pull request Sep 8, 2024
@skirpichev
redo on top ofpython#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-appbot commented Sep 8, 2024

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

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

bedevere-appbot commented Sep 8, 2024

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

@bedevere-appbedevere-appbot removed the needs backport to 3.12 only 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>
Yhg1s pushed a commit that referenced this pull request Sep 30, 2024
@skirpichev
[3.13]gh-121039: add Floats/ComplexesAreIdenticalMixin to test.suppo…
62e54dd
…rt.testcase (GH-121071) (#123840) * [3.13] 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> * +1
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchakaserhiy-storchakaserhiy-storchaka approved these changes

@sobolevnsobolevnsobolevn approved these changes

Assignees

@serhiy-storchakaserhiy-storchaka

Labels

skip newstestsTests 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