-
-
Couldn't load subscription status.
- Fork 33.3k
gh-128051: fix tests if sys.float_repr_style is 'legacy' #135908
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.
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
Conversation
|
CC @vstinner Second commit contains more controversial changes, i.e. where short and legacy repr - agree (but this tested on IEEE box!) |
| self.assertRegex(repr(c_ulonglong.from_param(20000)), r"^<cparam '[LIQ]' \(20000\)>$") | ||
| self.assertEqual(repr(c_float.from_param(1.5)), "<cparam 'f' (1.5)>") | ||
| self.assertEqual(repr(c_double.from_param(1.5)), "<cparam 'd' (1.5)>") | ||
| self.assertEqual(repr(c_double.from_param(1e300)), "<cparam 'd' (1e+300)>") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't remove tests. Maybe skip it if if getattr(sys, 'float_repr_style', '') == 'short':.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any idea why this was added, given we have other c_double test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test was added by commit 916610e.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I know. As well, as other tests in this function, but why? This value fits in double? We have only one test for float/longdouble.
|
We usually backport tests changes to stable branches to reduce the risk of merge conflicts for following tests changes. |
Co-authored-by: Victor Stinner <[email protected]>
| WorseStruct().__setstate__({}, b'foo') | ||
|
|
||
| @unittest.skipUnless(sys.float_repr_style == 'short', | ||
| "applies only when using short float repr style") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to only skip 1e300 test, rather than skipping the whole test method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Thanks @skirpichev for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14. |
…nGH-135908) (cherry picked from commit f3aec60) Co-authored-by: Sergey B Kirpichev <[email protected]> Co-authored-by: Victor Stinner <[email protected]>
|
Sorry, @skirpichev and @vstinner, I could not cleanly backport this to |
|
GH-136025 is a backport of this pull request to the 3.14 branch. |
|
GH-136026 is a backport of this pull request to the 3.13 branch. |
…n#135908) Co-authored-by: Victor Stinner <[email protected]> (cherry picked from commit f3aec60)
…35908) (#136025) gh-128051: Fix tests if sys.float_repr_style is 'legacy' (GH-135908) (cherry picked from commit f3aec60) Co-authored-by: Sergey B Kirpichev <[email protected]> Co-authored-by: Victor Stinner <[email protected]>
) (#136026) gh-128051: Fix tests if sys.float_repr_style is 'legacy' (#135908) (cherry picked from commit f3aec60) Co-authored-by: Sergey B Kirpichev <[email protected]>
…n#135908) Co-authored-by: Victor Stinner <[email protected]>
…n#135908) Co-authored-by: Victor Stinner <[email protected]>
…n#135908) Co-authored-by: Victor Stinner <[email protected]>
…n#135908) Co-authored-by: Victor Stinner <[email protected]>
Uh oh!
There was an error while loading. Please reload this page.