Skip to content

Return numpy scalars from numpy formatting and keep duration dtypes - #8519

Open
Kayvan-Zahiri wants to merge 1 commit into
huggingface:mainfrom
Kayvan-Zahiri:fix/8500-numpy-scalars-and-duration
Open

Return numpy scalars from numpy formatting and keep duration dtypes#8519
Kayvan-Zahiri wants to merge 1 commit into
huggingface:mainfrom
Kayvan-Zahiri:fix/8500-numpy-scalars-and-duration

Conversation

@Kayvan-Zahiri

Copy link
Copy Markdown

Closes #8500.

Two problems in NumpyFormatter, both falling out of numpy's type hierarchy.

Scalars. 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_). Both checks now use np.generic, which
covers every numpy scalar; np.character and np.number are already subclasses
of it, so the two existing branches collapse into it cleanly.

Durations. np.issubdtype(np.dtype("timedelta64[us]"), np.integer) is
True, so the int64 default dtype applied to duration columns. That is why the
same column reported timedelta64 by row and int64 by batch. The default now
skips timedelta64.

Before and after, on {"b": True, "t": datetime, "d": timedelta, "i": 3}:

row    b: ndarray(0-d, unhashable) -> np.bool_
       t: ndarray(0-d, unhashable) -> np.datetime64
batch  d: int64                    -> timedelta64[us]

int64 and float32 defaults for ordinary integer and float columns are
unchanged, and so are strings.

Two tests added, both failing without the change. tests/test_formatting.py,
tests/test_arrow_dataset.py and tests/test_table.py stay at 664 passed with
the same single pre-existing failure (test_dataset_to_iterable_dataset, which
fails identically on a clean checkout here). ruff check and ruff format
clean.

Separate from #8518, which touches table_cast.

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
@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

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 ebarkhordar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#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.

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

Labels

None yet

3 participants