Repository navigation
fix: read model text files as UTF-8 regardless of platform locale - #786
Conversation
open() without an encoding uses the locale encoding, which is cp1252 on
Windows. Model files are UTF-8, so Bm25(language=...) raised
UnicodeDecodeError for Russian, Greek and Arabic stopwords and silently
garbled German and French ones ("über" read as "über", so it was never
filtered). The same applied to vocab files, IDF files and tokenizer and
preprocessor configs.
Pass encoding="utf-8" to every text-mode open() in the package.
|
No actionable comments were generated in the recent review. ℹ️ Recent review info
Priority: Change: Bug fix Merge Risk:
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 9.52% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 8 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
Check name Status Explanation Title check ✅ Passed The title clearly summarizes the main change: reading model text files as UTF-8 regardless of platform locale. Description check ✅ Passed The description explains the Windows decoding issue, the files covered by the fix, and the tests used to validate it. Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request. Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
|
Hi, friendly ping on this one and on #785 (tests for the image transform helpers). Both are small and ready for review whenever someone has a moment. Happy to adjust anything. Thanks! |
open()withoutencoding=uses the locale encoding, which is cp1252 on Windows. Model files are UTF-8, so on Windows:SparseTextEmbedding("Qdrant/bm25", language="russian")(alsogreek,arabic) fails at init withUnicodeDecodeError: 'charmap' codec can't decode byte 0x8f.germanandfrenchload, but every stopword with an accent comes out garbled (überbecomesüber), so those stopwords are never filtered.The same bare
open()is used for miniCOIL/BM42 stopwords, the IF-SPLADE IDF file,VocabResolvervocab files, and the tokenizer/preprocessor JSON configs. Those configs can contain non-ASCII text too, e.g. SentencePiece's▁. This PR passesencoding="utf-8"to all 15 text-modeopen()calls in the package. Nothing changes on Linux/macOS, where the locale is already UTF-8.Reproduced on Windows 11 / Python 3.12 against the files in
Qdrant/bm25:With the fix,
language="russian"loads 151 stopwords, and"Das ist über alles"withlanguage="german"is all stopwords (empty embedding), as expected.Tests:
tests/test_text_file_encoding.pycovers BM25 stopwords in five languages,VocabResolvertxt/JSON round trips with non-ASCII words, andload_special_tokenswith▁. The test matrix runs onwindows-latest, so these guard the regression there.Note: #684 makes the same one-line change in
Bm25._load_stopwordsas part of Persian support, so whichever lands second has a trivial conflict on that line.