Repository navigation
refactor: import UniversalDetector from chardet top-level - #1215
jellyfishing2346 wants to merge 1 commit into
Conversation
|
Hi @leos, I was wondering if you happen to have any feedback on this PR. |
|
@jellyfishing2346 does this still support backwards compatibility? If reqs don't require the new version then this could break older versions. |
|
@JGSweets it doesn’t. |
|
@jellyfishing2346 do you know if they ever resolved this issue of changing the default likelihood: Also, unsure if they resolved their license issue for 7.0.0+ |
|
@JGSweets I don't believe I know if they did. |
424191f to
8c15364
Compare
Avoids the deprecated chardet.universaldetector path in chardet 7.x. The top-level import works on older versions too. Refs capitalone#1204
8c15364 to
38d8568
Compare
|
@JGSweets Correction to my earlier answer: this change is backwards compatible. I tested from chardet import UniversalDetector against the full range allowed in requirements.txt (>=3.0.4,<7.0.0): 3.0.4, 4.0.0, 5.2.0 and 6.0.0. It works on all of them, and on 7.6.0 as well. On your other questions: I haven't seen the utf-8 → windows-1252 detection change or the 7.x licensing dispute (chardet/chardet#327) resolved, so I don't think this PR should lift the <7.0.0 pin. I've rescoped it as a small forward-compat cleanup instead: the old chardet.universaldetector path is deprecated in 7.x, so this avoids a future break if the pin is ever relaxed. I've rebased on the latest main and updated the title. If you'd rather not take it given the pin, I'm happy to close it. |
Changed from 'from chardet.universaldetector import UniversalDetector' to 'from chardet import UniversalDetector' to support chardet 0.7.0+ which changed their import structure.
Fixes #1204