fix: honor encoding= in partition_md - #4488
Open
linhongyu510 wants to merge 1 commit into
Open
linhongyu510 wants to merge 1 commit into
linhongyu510 wants to merge 1 commit into
Conversation
`partition_md()` reads the source with `read_txt_file()` but never passes an
encoding, and has no `encoding` parameter of its own -- so a caller's codec is
swallowed by `**kwargs`, which the docstring documents as forwarded to
`partition_html`.
Charset detection covers the easy cases, so this is invisible for a UTF-8 or
BOM-bearing UTF-16 document. A legacy single-byte codepage is where it bites:
partition_md(filename="cp1252.md") -> "Café naďve sección"
partition_text(filename="cp1252.txt", encoding="cp1252")
-> "Café naïve sección"
The same bytes are recoverable through `partition_text()` because it takes
`encoding`; through `partition_md()` there is no way to express it and the
mojibake is final.
Accept `encoding` and forward it to both `read_txt_file()` call sites. The
parameter is added after the existing ones so current positional calls keep
binding as before, and omitting it preserves the detecting behavior exactly.
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_md()reads the source throughread_txt_file()without passing an encoding, and has noencodingparameter of its own:so a caller's codec lands in
**kwargs, which the docstring documents as forwarded topartition_html— and is silently dropped for the read that actually needs it.Charset detection covers the easy cases, which is why this stays invisible for a long time. I checked the whole family on
main(3376cc96) against a cp1252 document:encoding=partition_textCafé naïve secciónpartition_mdCafé naďve secciónUTF-8 and BOM-bearing UTF-16 decode correctly with or without the argument, so they do not expose the gap — a legacy single-byte codepage does. The same bytes are recoverable through
partition_text()because it acceptsencoding; throughpartition_md()there is no way to express it and the mojibake is final.Fix
Accept
encodingand forward it to bothread_txt_file()call sites.The parameter is added after the existing ones, so current positional calls keep binding exactly as before. Omitting it preserves the detecting behavior unchanged.
Tests
test_partition_md_honors_an_explicit_encoding_for_a_filenametest_partition_md_honors_an_explicit_encoding_for_a_file_like_objecttest_partition_md_still_detects_the_encoding_when_none_is_given— guards the unchanged defaultThe tests use cp1252 deliberately: a UTF-16 payload would pass even if the argument were ignored, so it could not tell the fix from the bug.
Verification
test_md.py(39 pre-existing + 3 new).encoding=pass-throughs while keeping the parameter and the tests fails exactly the 2 "honors encoding" tests, with the "still detects" test still passing — the tests separate the new capability from the preserved default.test_md.py,test_text.py,test_json.pyandtest_ndjson.pythe failure set is identical before and after; passing count goes 219 → 222, exactly the 3 added tests. (3 pre-existing failures are environmental and unrelated to this change.)ruff checkandruff format --checkclean on both changed files.partition/md.py: none touches it. Compatibility Issue with Chinese Text in Document Parsing #3530 and Compatibility Issue with Chinese Text in Document Parsing #3267 modifytest_md.pyandpartition/text.pybut change sentence-splitting for Chinese text, not encoding — and both have been idle since 2024.Separate from #4483, which covers
partition_json/partition_ndjson; this branch is cut from currentmainand the two do not overlap.