Keep integers on the python read path for fixed-shape ArrayXD columns with nulls#8363
Open
ebarkhordar wants to merge 1 commit into
Open
Conversation
… with nulls A fixed-shape ArrayXD column that contains a null row was silently cast to float on the python read path (to_dict / to_pylist): non-null integers came back as floats and values above 2**53 lost precision, because an unrelated row was null. Build the python list from the nested-list storage in that case, which keeps the exact values and returns None for the null rows. The numpy format path is unchanged (to_numpy still returns the dense float64 array with np.nan for nulls), and the dynamic-first-dim branch is untouched.
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
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.
What does this PR do?
Fixes #8362.
A fixed-shape
ArrayXDcolumn (Array2D(shape=(1, 2), dtype="int64"), and so on) that contains aNonerow is silently cast to float on the python read path:to_dict()/to_pylist()return every value as a float, and integers above 2**53 are altered, because an unrelated row is null.dataset[i][column]returns the correct integers, so the result depends on whether another row happens to be null.Root cause
ArrayExtensionArray.to_numpycasts a fixed-shape column tofloat64so it can insertnp.nanfor the null rows (a dense numpy array cannot hold both integers and a missing marker).to_pylistreturnsnumpy_arr.tolist(), so that float cast reaches the python read path, whereNoneis available and no cast is needed.Fix
In
to_pylist, when the array is fixed-shape and has nulls, build the list from the nested-liststorage(self.storage.to_pylist()), which already carries the exact values andNonefor the null rows.The invariant: a non-null value read through the python path must never be altered because another row in the column is null.
Scope kept deliberately narrow:
to_numpystill returns the densefloat64+np.nanarray (asserted by the existingtest_array_xd_with_none), sowith_format("numpy")behaves exactly as before.shape=(None, ...)) is untouched.Verification
test_array_xd_with_none_python_format_keeps_integers: fails onmain([[nan, nan]] != None), passes here. It covers integer preservation, the 2**53 + 1 precision case, and asserts the numpy format still returns a densefloat64+nanarray.python -m pytest tests/features/test_array_xd.py: 103 passed.ruff checkandruff format --checkclean on the changed files.python:3.11-slimcontainer at HEAD 521a590. Not verified: the torch/tf/jax formatting paths, which this change does not touch (it only affects the pythonto_pylistpath).