search

matthewwithanm / django-imagekit

Automated image processing for Django. Currently v4.0
http://django-imagekit.rtfd.org/
BSD 3-Clause "New" or "Revised" License
2.24k stars 276 forks source link

Import force/smart_str instead of force/smart_text #534

Closed jaap3 closed 2 years ago

jaap3 commented 2 years ago

force_text and smart_text were deprecated in Django 3.0 and are removed in Django 4.0

jaap3 commented 2 years ago

force_str and smart_str are available since at least Django 1.8.

This doesn't seem like the cleanest fix, and it feels like the backwards compatibility with Django 1.5 might not be necessary. But I'm unsure of the Django/Python support policy of this project. I noticed that the tox and travis configs only test fairly old Django versions, which surprised me.

vstoykov commented 2 years ago

force_text is needed for Python 2 compatibility on places where we know with sure that we need text (unicode). If we drop python 2 support we can use force_str only. Based on the discussion in #535 I'm suggesting if you are willing you can make a PR to drop Python 2 and remove all compatibility shims for it.

vstoykov commented 2 years ago

Because after the merge of #536 we are testing Django 2.2+ and Python 3.6+ we can assume that we can remove some compatibility shims in order to cleanup the code base. Do you think that this PR is a good candidate for that or another one which will do a full dissection of the whole project?

jaap3 commented 2 years ago

@vstoykov I'd rather get this merged and a patch release on PyPI before gutting all the historic compatibility code.

vstoykov commented 2 years ago

If we do a patch release this means that it should support Python 2 and proposed changes will probably break something on Python 2. This is my gut feeling because I'm not sure that it is tested correctly.

With the changes that are already merged we do not test old versions of Django and Python. This means that we are droping support for that. If we are dropping the support probably now is the time to cleanup the code base and release version 5 of ImageKit. What you think?

jaap3 commented 2 years ago

I've added Python 2.7 back to the tests and restored compatibility (in the tests only, the codebase remained compatible). Had to pin django-appconf for Python 2.7 as the recent release is not compatible with Python 2.

Django 1.11 is also the only version tested on Python 2.7 as future Django versions dropped Python 2 support.