gh-154568: Fix array._array_reconstructor() ignoring requested byte order for float16#154569
Open
PhysicistJohn wants to merge 3 commits into
Open
gh-154568: Fix array._array_reconstructor() ignoring requested byte order for float16#154569PhysicistJohn wants to merge 3 commits into
PhysicistJohn wants to merge 3 commits into
Conversation
…byte order for float16
The slow-path decoder for IEEE_754_FLOAT16_LE/BE computed the
byte-order flag by comparing mformat_code against IEEE_754_FLOAT_LE
(the 32-bit float constant) instead of IEEE_754_FLOAT16_LE. Since the
float16 mformat codes are never equal to that constant, the comparison
was always false, so the decoder always treated input as big-endian
regardless of what was requested.
Adds a regression test. A real 'e'-typecode array only exercises the
slow/converting path when the requested mformat disagrees with the
machine's native float16 format, so which direction (LE/BE) exercises
the buggy branch depends on the test machine's endianness, and the
other direction happens to come out "correct" by coincidence since the
bug unconditionally decodes as big-endian. The test uses a typecode
('d') whose native format can never match the float16 codes, forcing
the slow path deterministically on any machine.
This comment was marked as resolved.
This comment was marked as resolved.
…ction array._array_reconstructor is internal/private and has no Sphinx docs entry, so the :func: cross-reference role can't resolve and trips the docs CI's new-NEWS-nit check.
skirpichev
self-requested a review
July 24, 2026 03:12
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
skirpichev
approved these changes
Jul 24, 2026
| msg="{0!r} != {1!r}; testcase={2!r}".format(a, b, testcase)) | ||
|
|
||
| def test_float16_endianness(self): | ||
| # gh-issue: the slow-path decoder for IEEE_754_FLOAT16_LE/BE |
Member
There was a problem hiding this comment.
Suggested change
| # gh-issue: the slow-path decoder for IEEE_754_FLOAT16_LE/BE | |
| # gh-154568: the slow-path decoder for IEEE_754_FLOAT16_LE/BE |
Second thought, I think this test is fine. Just please add half-floats case to the test_numbers().
skirpichev
added
the
needs backport to 3.15
Member
|
CC @vstinner |
…lify NEWS wording Per skirpichev's review on pythonGH-154569: - Add a float16 (LE/BE) case to ArrayReconstructorTest.test_numbers() for basic functional coverage alongside the other numeric types, complementing the existing dedicated test_float16_endianness (which he confirmed is fine to keep for deterministically forcing the buggy slow path regardless of test-machine endianness). - Simplify the NEWS entry wording per his suggested rewrite.
Author
|
Thanks for the review! Pushed both changes: added a half-floats case to |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The slow-path decoder for IEEE_754_FLOAT16_LE/BE compared mformat_code
against IEEE_754_FLOAT_LE (the 32-bit float constant) instead of
IEEE_754_FLOAT16_LE, so it always decoded as big-endian regardless of
what was requested. Fixes gh-154568.
Adds a regression test. A real 'e'-typecode array only exercises the
slow/converting path when the requested mformat disagrees with the
machine's native float16 format, so which direction (LE/BE) actually
exercises the buggy branch depends on the test machine's endianness --
and the other direction happens to come out "correct" by coincidence,
since the bug unconditionally decodes as big-endian. The test uses a
typecode ('d') whose native format can never match the float16 codes,
which forces the slow path deterministically on any machine so both
directions are actually exercised.