Skip to content

fix: correctly decode numpy UCS-4 arrays instead of treating as UTF-8 - #2386

Open
VirajMishra1 wants to merge 1 commit into
huggingface:mainfrom
VirajMishra1:fix-numpy-ucs4-decoding
Open

VirajMishra1 wants to merge 1 commit into
huggingface:mainfrom
VirajMishra1:fix-numpy-ucs4-decoding

Conversation

@VirajMishra1

Copy link
Copy Markdown

PyArrayUnicode in the Python bindings passes raw numpy dtype='U' array bytes directly to std::str::from_utf8(). But numpy stores Unicode strings as UCS-4 (UTF-32LE on little-endian), not UTF-8. This causes:

  • ASCII strings: silent data corruption -- embedded null bytes between every character (e.g. "hello" becomes "h\0\0\0e\0\0\0l\0\0\0l\0\0\0o\0\0\0")
  • Non-ASCII strings: ValueError('invalid utf-8 sequence') for any input containing accented characters, CJK, emoji, etc.

The commented-out code directly below the broken path (using PyUnicode_FromKindAndData with PyUnicode_4BYTE_KIND) was the correct approach but was replaced with the from_utf8 call.

Fix

Decode 4-byte chunks as UTF-32LE code points (u32::from_le_bytes), filter null padding, convert via char::from_u32, and collect to String. This correctly handles numpy's UCS-4 encoding for all Unicode inputs.

Test

Added test_numpy_unicode_ucs4 in bindings/python/tests/bindings/test_tokenizer.py covering ASCII, CJK, and emoji inputs through numpy dtype='U' arrays.

PyArrayUnicode passed numpy dtype='U' array bytes directly to
std::str::from_utf8(), but numpy stores Unicode strings as UCS-4
(UTF-32LE on little-endian systems), not UTF-8.

This caused:
- ASCII strings: silent data corruption (embedded null bytes)
- Non-ASCII strings: ValueError('invalid utf-8 sequence')

Fix: decode 4-byte chunks as UTF-32LE code points and collect to
String, properly handling the UCS-4 encoding numpy uses.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant