fix: honor encoding= in partition_json and partition_ndjson - #4483
Open
linhongyu510 wants to merge 1 commit into
Open
fix: honor encoding= in partition_json and partition_ndjson#4483linhongyu510 wants to merge 1 commit into
linhongyu510 wants to merge 1 commit into
Conversation
Both partitioners read the source with a hard-coded UTF-8:
with open(filename, encoding="utf8") as f:
file_text = f.read()
and, for a file-like object, a bare .decode() (also UTF-8). Neither
accepted an `encoding` parameter, so an `encoding=` passed by the caller
-- directly or via partition() -- landed in **kwargs and was discarded
without a warning. A UTF-16 document raised UnicodeDecodeError even when
its encoding was stated explicitly:
partition_json(filename="utf16.json", encoding="utf-16")
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xff in position 0
partition_text() and partition_csv() both take `encoding` and read
through read_txt_file(), which uses the given encoding when present and
detects it otherwise. Route the two JSON partitioners through the same
helper and give them the same parameter.
Reading a non-UTF-8 document without stating its encoding now works too,
since read_txt_file() falls back to detection.
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.
Problem
partition_json()andpartition_ndjson()read the source with a hard-codedUTF-8, and neither accepts an
encodingparameter:Because both signatures end in
**kwargs: Any, anencoding=passed by thecaller — directly, or forwarded by
partition()— is swallowed silently. Onmain(ee2b3a35):The same call succeeds for the sibling text formats, which is where the
inconsistency shows:
encodingpartition_textread_txt_file()partition_csvpartition_jsonopen(..., encoding="utf8")partition_ndjsonopen(..., encoding="utf8")UTF-16 JSON is not exotic — it is what several Windows tools emit by default,
and
json.dumpto a file opened in a non-UTF-8 encoding produces it too.Fix
Add
encodingto both signatures and read through the existingread_txt_file()helper, exactly aspartition_text()does:read_txt_file()uses the given encoding when one is supplied and falls back todetect_file_encoding()when it is not, so this also makes a non-UTF-8 documentreadable without naming its encoding. The
text=path is untouched, andfile.seek(0)is preserved so the detect-then-partition sequence over one handlestill works.
Source change is +11/-8 across the two files; no new helper, no behaviour change
for UTF-8 input.
Verification
The 35 failures are pre-existing on
mainin this environment — they come fromunstructured/nlp/tokenize.pyfailing to import an optional dependency(
ModuleNotFoundError: No module named 'installer'), unrelated to these files.I compared the failure sets with
git stash: identical before and after, andthe pass count goes 70 → 75, which is exactly the five tests added here.
Five regression tests, following the existing naming style in both files:
encoding="utf-16"forfilename=(both partitioners)encoding="utf-16"forfile=(both partitioners)encodinggiven, UTF-16 detected (json)Rollback proof. Restoring just the two read blocks to the hard-coded form —
leaving the new parameter in place — fails exactly those five and nothing else:
each with
UnicodeDecodeError.ruff checkon the four changed files is clean,and a CHANGELOG entry is added under the current
0.27.8-dev0section.Notes
This is the parsing side. There are open PRs (#4397, #4398, #4447) addressing
non-UTF-8 crashes during file-type detection in
file_utils/filetype.py; Ichecked all currently open PRs and none touches
partition/json.pyorpartition/ndjson.py, so this does not overlap with them.I kept the scope to honoring the parameter. I did not change what happens when a
document really is undecodable, nor touch the
text=path.