Add official support for Django 2.2. #46

joehybird · 2020-01-23T05:15:52Z

No description provided.

coveralls · 2020-01-23T05:23:22Z

Coverage increased (+2.0%) to 98.901% when pulling 3745f2f on joehybird:master into 65afd88 on gintas:master.

joehybird · 2020-01-23T09:58:13Z

Why coverage from 95% (master) to 97% (this branch) is show as decreased ?

charettes

Hey @joehybird!

Thank you for your patch but it seems to be doing more than simply adding support for Django 2.2.

Most of the changes you propose make sense but the fact they are all bundled together under a Add official support for Django 2.2. Improve coverage commit doesn't.

Could you break down this single commit into multiple ones that do the following:

Switch TravisCI configuration to use bionic dist.
Improve coverage reporting.
Add support for Django 2.2.
Drop support for Django 2.0 and 2.1.

charettes · 2020-01-23T15:14:07Z

tests/tests.py

@@ -35,6 +40,7 @@ def test_data_integrity(self):
            # the same data, even thought it's stored differently in the DB.
            self.assertEqual(value, model_test.pickle_field)
            self.assertEqual(value, model_test.compressed_pickle_field)
+            self.assertEqual(None, model_test.nullable_pickle_field)


Suggested change

self.assertEqual(None, model_test.nullable_pickle_field)

self.assertIsNone(model_test.nullable_pickle_field)

good catch :)

charettes · 2020-01-23T15:14:52Z

picklefield/fields.py

-    if django.VERSION < (2, 0):
-        def from_db_value(self, value, expression, connection, context):
-            return self.to_python(value)
-    else:
-        def from_db_value(self, value, expression, connection):
-            return self.to_python(value)
+    def from_db_value(self, value, expression, connection, context=None):
+        return self.to_python(value)


This is still necessary unless you drop support for Django 1.11.

It works in all versions with full coverage, but the django 3.0 warning is back. Pretty strange because master (3.1) is ok.

@joehybird it will work but the idea is to avoid the warning on Django 3.0.

The context argument is required on Django < 2.0 and will result in a warning if present on Django < 3.1.

Please restore the previous behavior.

charettes · 2020-01-23T15:15:55Z

tox.ini

-    py35-{1.11,2.0,2.1,master},
-    py36-{1.11,2.0,2.1,master},
-    py37-{1.11,2.0,2.1,master}
+    py{27,36,37,38}-1.11,


1.11 doesn't support 3.7 and 3.8, see https://djangoci.com/job/django-1.11/

charettes · 2020-01-23T15:17:30Z

tox.ini

-    py36-{1.11,2.0,2.1,master},
-    py37-{1.11,2.0,2.1,master}
+    py{27,36,37,38}-1.11,
+    py{36,37,38}-{2.0,2.1,2.2,master}


2.0 and 2.1 are unsupported

2.2 supports 3.5 as well https://djangoci.com/job/django-2.2/

charettes · 2020-01-23T15:17:49Z

tox.ini


 [testenv]
 basepython =
    py27: python2.7
-    py34: python3.4
-    py35: python3.5


The 2.2 LTS supports 3.5

charettes · 2020-01-23T15:18:15Z

tests/tests.py

+    def test_decode_error(self):
+        def mock_decode_error(*args, **kwargs):
+            raise Exception()
+
+        model = MinimalTestingModel.objects.create(pickle_field={'foo': 'bar'})
+        model.save()
+
+        self.assertEqual(
+            {'foo': 'bar'}, MinimalTestingModel.objects.get(pk=model.pk).pickle_field
+        )
+
+        with patch('picklefield.fields.dbsafe_decode', mock_decode_error):
+            encoded_value = dbsafe_encode({'foo': 'bar'})
+            self.assertEqual(encoded_value, MinimalTestingModel.objects.get(pk=model.pk).pickle_field)
+


What does this patch have to do with Django 2.2 support?

Nothing, just coverage improvement 😄

joehybird · 2020-01-23T16:14:59Z

Ok I will split it in different commits (should be easy... I hope so...)

charettes

Thanks for the extra mile, please revert the from_db_value changes and I'll proceed with the merge.

charettes · 2020-01-24T14:59:42Z

picklefield/fields.py

-    if django.VERSION < (2, 0):
-        def from_db_value(self, value, expression, connection, context):
-            return self.to_python(value)
-    else:
-        def from_db_value(self, value, expression, connection):
-            return self.to_python(value)
+    def from_db_value(self, value, expression, connection, context=None):
+        return self.to_python(value)


@joehybird it will work but the idea is to avoid the warning on Django 3.0.

The context argument is required on Django < 2.0 and will result in a warning if present on Django < 3.1.

Please restore the previous behavior.

joehybird · 2020-01-27T13:42:57Z

Ok fixed :)

charettes · 2020-01-27T15:04:04Z

Awesome thanks for everything!

joehybird · 2020-01-28T11:33:41Z

@charettes You can thank me with new release 🙏

charettes · 2020-01-28T15:22:36Z

Et voilà https://pypi.org/project/django-picklefield/

joehybird force-pushed the master branch from 2ca98f6 to da768d5 Compare January 23, 2020 05:19

joehybird force-pushed the master branch 3 times, most recently from 07c9314 to 81c8c54 Compare January 23, 2020 08:12

joehybird requested a review from charettes January 23, 2020 09:56

joehybird marked this pull request as ready for review January 23, 2020 09:56

joehybird force-pushed the master branch 3 times, most recently from f5f9c5a to 8037548 Compare January 23, 2020 14:28

joehybird requested a review from gintas January 23, 2020 15:11

charettes reviewed Jan 23, 2020

View reviewed changes

Improve travis script & switch to bionic beaver (python 3.8)

3775860

joehybird force-pushed the master branch 2 times, most recently from b4224fd to 72b90d2 Compare January 24, 2020 05:09

joehybird requested a review from charettes January 24, 2020 05:47

charettes approved these changes Jan 24, 2020

View reviewed changes

joehybird added 4 commits January 27, 2020 06:01

Improve coverage reporting

728d084

Add support for Django 2.2

e0da79b

Drop support for Django 2.0 & 2.1

e21c854

Drop support of python 3.4

3745f2f

joehybird force-pushed the master branch from 4226edd to 3745f2f Compare January 27, 2020 05:02

joehybird requested a review from charettes January 27, 2020 13:42

charettes merged commit 31a4a64 into gintas:master Jan 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add official support for Django 2.2. #46

Add official support for Django 2.2. #46

joehybird commented Jan 23, 2020

coveralls commented Jan 23, 2020 •

edited

joehybird commented Jan 23, 2020

charettes left a comment

charettes Jan 23, 2020

joehybird Jan 23, 2020

charettes Jan 23, 2020

joehybird Jan 23, 2020

charettes Jan 24, 2020

charettes Jan 23, 2020

charettes Jan 23, 2020

charettes Jan 23, 2020

charettes Jan 23, 2020

joehybird Jan 23, 2020

joehybird commented Jan 23, 2020

charettes left a comment

charettes Jan 24, 2020

joehybird commented Jan 27, 2020

charettes commented Jan 27, 2020

joehybird commented Jan 28, 2020 •

edited

charettes commented Jan 28, 2020

	self.assertEqual(None, model_test.nullable_pickle_field)
	self.assertIsNone(model_test.nullable_pickle_field)

Add official support for Django 2.2. #46

Add official support for Django 2.2. #46

Conversation

joehybird commented Jan 23, 2020

coveralls commented Jan 23, 2020 • edited

joehybird commented Jan 23, 2020

charettes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joehybird commented Jan 23, 2020

charettes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joehybird commented Jan 27, 2020

charettes commented Jan 27, 2020

joehybird commented Jan 28, 2020 • edited

charettes commented Jan 28, 2020

coveralls commented Jan 23, 2020 •

edited

joehybird commented Jan 28, 2020 •

edited