Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.2k
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
Second commit contains more controversial changes, i.e. where short and legacy repr - agree (but this tested on IEEE box!) |
| @@ -240,7 +240,6 @@ def test_parameter_repr(self): | |||
| 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 ifif 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 otherc_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 commit916610e.
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.
Uh oh!
There was an error while loading.Please reload this page.
We usually backport tests changes to stable branches to reduce the risk of merge conflicts for following tests changes. |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Victor Stinner <vstinner@python.org>
| @@ -224,6 +225,8 @@ def __dict__(self): | |||
| with self.assertRaises(ZeroDivisionError): | |||
| 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
f3aec60 intopython:mainUh oh!
There was an error while loading.Please reload this page.
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 commitf3aec60)Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>Co-authored-by: Victor Stinner <vstinner@python.org>
Sorry,@skirpichev and@vstinner, I could not cleanly backport this to |
GH-136025 is a backport of this pull request to the3.14 branch. |
GH-136026 is a backport of this pull request to the3.13 branch. |
Uh oh!
There was an error while loading.Please reload this page.