Skip to content

Conversation

@skirpichev
Copy link
Member

@skirpichevskirpichev commented Nov 2, 2023

@bedevere-appbedevere-appbot mentioned this pull request Nov 2, 2023
10 tasks
@skirpichev
Copy link
MemberAuthor

Some C API tests for floats are in Lib/test/test_float.py historically. An example:

@unittest.skipIf(_testcapiisNone, 'needs _testcapi')
classPackTests(unittest.TestCase):
deftest_pack(self):
self.assertEqual(_testcapi.float_pack(2, 1.5, BIG_ENDIAN),
b'>\x00')
self.assertEqual(_testcapi.float_pack(4, 1.5, BIG_ENDIAN),
b'?\xc0\x00\x00')
self.assertEqual(_testcapi.float_pack(8, 1.5, BIG_ENDIAN),
b'?\xf8\x00\x00\x00\x00\x00\x00')
self.assertEqual(_testcapi.float_pack(2, 1.5, LITTLE_ENDIAN),
b'\x00>')
self.assertEqual(_testcapi.float_pack(4, 1.5, LITTLE_ENDIAN),
b'\x00\x00\xc0?')
self.assertEqual(_testcapi.float_pack(8, 1.5, LITTLE_ENDIAN),
b'\x00\x00\x00\x00\x00\x00\xf8?')
deftest_unpack(self):
self.assertEqual(_testcapi.float_unpack(b'>\x00', BIG_ENDIAN),
1.5)
self.assertEqual(_testcapi.float_unpack(b'?\xc0\x00\x00', BIG_ENDIAN),
1.5)
self.assertEqual(_testcapi.float_unpack(b'?\xf8\x00\x00\x00\x00\x00\x00', BIG_ENDIAN),
1.5)
self.assertEqual(_testcapi.float_unpack(b'\x00>', LITTLE_ENDIAN),
1.5)
self.assertEqual(_testcapi.float_unpack(b'\x00\x00\xc0?', LITTLE_ENDIAN),
1.5)
self.assertEqual(_testcapi.float_unpack(b'\x00\x00\x00\x00\x00\x00\xf8?', LITTLE_ENDIAN),
1.5)
deftest_roundtrip(self):
large=2.0**100
values= [1.0, 1.5, large, 1.0/7, math.pi]
ifHAVE_IEEE_754:
values.extend((INF, NAN))
forvalueinvalues:
forsizein (2, 4, 8,):
ifsize==2andvalue==large:
# too large for 16-bit float
continue
rel_tol=EPSILON[size]
forendianin (BIG_ENDIAN, LITTLE_ENDIAN):
withself.subTest(value=value, size=size, endian=endian):
data=_testcapi.float_pack(size, value, endian)
value2=_testcapi.float_unpack(data, endian)
ifisnan(value):
self.assertTrue(isnan(value2), (value, value2))
elifsize<8:
self.assertTrue(math.isclose(value2, value, rel_tol=rel_tol),
(value, value2))
else:
self.assertEqual(value2, value)

In this PR this left intact, but maybe we should move them.

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.

Just one suggestion :)
All other things LGTM.

@skirpichev
Copy link
MemberAuthor

Converted to a draft per review #111591

@skirpichevskirpichev marked this pull request as ready for review November 4, 2023 15:48
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.

Thank you @skirpichev for your contribution and for your fast response.

@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 pushed a commit to miss-islington/cpython that referenced this pull request Nov 5, 2023
(cherry picked from commit b452202) Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
@bedevere-app
Copy link

GH-111752 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 Nov 5, 2023
@skirpichevskirpichev deleted the capi-float-tests branch November 5, 2023 07:21
serhiy-storchaka pushed a commit that referenced this pull request Nov 5, 2023
(cherry picked from commit b452202) Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
@skirpichev
Copy link
MemberAuthor

@serhiy-storchaka, thank you for review.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip newstestsTests in the Lib/test dirtopic-C-API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@skirpichev@serhiy-storchaka@sobolevn