Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to chardet 4.x #5688

Merged
merged 1 commit into from
Dec 14, 2020
Merged

Upgrade to chardet 4.x #5688

merged 1 commit into from
Dec 14, 2020

Conversation

dan-blanchard
Copy link
Contributor

@dan-blanchard dan-blanchard commented Dec 11, 2020

I just released chardet 4.0.0 today, and it's faster and fully backward compatible with chardet 3.x (as long as you aren't mucking around in the models it uses under-the-hood directly). The next major release will be Python 3.6+, but seeing as it took me three years to put out this one, that's unlikely to be soon.

@maxice8
Copy link

maxice8 commented Dec 11, 2020

requests/__init__.py lines 69 to 71 need to be updated otherwise one gets a warning:

/usr/lib/python3.8/site-packages/requests/__init__.py:89: RequestsDependencyWarning: urllib3 (1.26.2) or chardet (4.0.0) doesn't match a supported version!
  warnings.warn("urllib3 ({}) or chardet ({}) doesn't match a supported "
assert major == 3
assert minor < 1
assert patch >= 2

@maxice8
Copy link

maxice8 commented Dec 11, 2020

I got this patch

diff --git a/setup.py b/setup.py
index 4ff0cfe..eca8bf5 100755
--- a/setup.py
+++ b/setup.py
@@ -42,7 +42,7 @@ if sys.argv[-1] == 'publish':
 packages = ['requests']
 
 requires = [
-    'chardet>=3.0.2,<4',
+    'chardet>=4.0.0,<5',
     'idna>=2.5,<3',
     'urllib3>=1.21.1,<1.27',
 
-- 
2.29.2

diff --git a/requests/__init__.py b/requests/__init__.py
index c00f556..11f0e97 100644
--- a/requests/__init__.py
+++ b/requests/__init__.py
@@ -66,9 +66,9 @@ def check_compatibility(urllib3_version, chardet_version):
     major, minor, patch = chardet_version.split('.')[:3]
     major, minor, patch = int(major), int(minor), int(patch)
     # chardet >= 3.0.2, < 3.1.0
-    assert major == 3
-    assert minor < 1
-    assert patch >= 2
+    assert major == 4
+    assert minor >= 0
+    assert patch >= 0
 
 
 def _check_cryptography(cryptography_version):

Copy link
Member

@nateprewitt nateprewitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this @dan-blanchard! @maxice8 is right we'll need to get the init checks updated as well. Let's also keep the existing floor (3.0.2) for chardet unless there's a strong reason to raise it.

Feel free to add those changes or we'll get them incorporated before merging.

@maxice8
Copy link

maxice8 commented Dec 11, 2020

This should be goodish enough (idk if you can test >= and < in the same assert though):

diff --git a/requests/__init__.py b/requests/__init__.py
index c00f556..9bfe511 100644
--- a/requests/__init__.py
+++ b/requests/__init__.py
@@ -65,10 +65,10 @@ def check_compatibility(urllib3_version, chardet_version):
     # Check chardet for compatibility.
     major, minor, patch = chardet_version.split('.')[:3]
     major, minor, patch = int(major), int(minor), int(patch)
-    # chardet >= 3.0.2, < 3.1.0
-    assert major == 3
-    assert minor < 1
-    assert patch >= 2
+    # chardet >= 3.0.2, < 5.0.0
+    assert major >= 3 < 5
+    assert minor >= 0
+    assert patch >= 0
 
 
 def _check_cryptography(cryptography_version):
diff --git a/setup.py b/setup.py
index e714bfa..7ba4b2a 100755
--- a/setup.py
+++ b/setup.py
@@ -42,7 +42,7 @@ if sys.argv[-1] == 'publish':
 packages = ['requests']
 
 requires = [
-    'chardet>=3.0.2,<4',
+    'chardet>=3.0.2,<5',
     'idna>=2.5,<3',
     'urllib3>=1.21.1,<1.27',
     'certifi>=2017.4.17'

@dan-blanchard
Copy link
Contributor Author

I've updated the check in requests/__init__.py

Copy link
Member

@nateprewitt nateprewitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @dan-blanchard! We'll work on getting a release out today or tomorrow.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants