Return numpy scalars from numpy formatting and keep duration dtypes - #8519
Return numpy scalars from numpy formatting and keep duration dtypes#8519Kayvan-Zahiri wants to merge 1 commit into
Conversation
Two problems in NumpyFormatter, both from numpy's type hierarchy. np.bool_ and np.datetime64 are not np.number, so _tensorize did not early return them and _recursive_tensorize ran them through __array__ first. Row access handed back 0-d ndarrays, which are unhashable and fail isinstance(x, np.bool_). Widening both checks to np.generic covers every numpy scalar, and np.character and np.number are both subclasses of it. np.issubdtype counts timedelta64 as an integer subtype, so the int64 default dtype applied to duration columns. The same column came back as timedelta64 by row and int64 by batch. The default now skips timedelta64. Closes huggingface#8500 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DnFJxiG7r2xeshM4G8rm3y
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
ebarkhordar
left a comment
There was a problem hiding this comment.
#8500 has three open PRs against src/datasets/formatting/np_formatter.py, so I fetched each head's copy of that file from this repo and diffed them.
#8503 5a97bc12 sha256 7210141c7c3c9f13
#8504 86105122 sha256 0cbf9e401c97e48c
#8519 1335ea15 sha256 99d8e045fc467e27
diff 8504 8519 -> 4 added lines, all comments (the np.generic note
and the timedelta64 note). No executable difference.
diff 8503 8504 -> 1 line. 8503 leaves _recursive_tensorize checking
(np.ndarray, np.character, np.number).
So the code change here is the same one #8504 makes, and #8503 is that change without the _recursive_tensorize line.
Where this PR does differ is the tests. It also asserts format_column, checks each scalar is not an ndarray, calls hash() on it, and covers a plain int column. #8504's single test asserts the three isinstance checks and the batch duration dtype only.
I have no view on which should land. Flagging it so the same diff does not get reviewed three times.
Closes #8500.
Two problems in
NumpyFormatter, both falling out of numpy's type hierarchy.Scalars.
np.bool_andnp.datetime64are notnp.number, so_tensorizedid not early-return them and
_recursive_tensorizeran them through__array__first. Row access handed back 0-dndarrays, which are unhashableand fail
isinstance(x, np.bool_). Both checks now usenp.generic, whichcovers every numpy scalar;
np.characterandnp.numberare already subclassesof it, so the two existing branches collapse into it cleanly.
Durations.
np.issubdtype(np.dtype("timedelta64[us]"), np.integer)isTrue, so the int64 default dtype applied to duration columns. That is why thesame column reported
timedelta64by row andint64by batch. The default nowskips timedelta64.
Before and after, on
{"b": True, "t": datetime, "d": timedelta, "i": 3}:int64andfloat32defaults for ordinary integer and float columns areunchanged, and so are strings.
Two tests added, both failing without the change.
tests/test_formatting.py,tests/test_arrow_dataset.pyandtests/test_table.pystay at 664 passed withthe same single pre-existing failure (
test_dataset_to_iterable_dataset, whichfails identically on a clean checkout here).
ruff checkandruff formatclean.
Separate from #8518, which touches
table_cast.