Skip to content

Import force/smart_str instead of force/smart_text #534

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

Merged
merged 2 commits into from
Nov 2, 2021

Conversation

jaap3
Copy link

@jaap3 jaap3 commented Sep 22, 2021

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

@jaap3
Copy link
Author

jaap3 commented Sep 22, 2021

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.

@jaap3 jaap3 changed the title Imoprt force/smart_str instead of force/smart_text Import force/smart_str instead of force/smart_text Sep 22, 2021
@vstoykov
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Author

jaap3 commented Oct 14, 2021

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

@vstoykov
Copy link
Collaborator

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?

force_text and smart_text were deprecated in Django 3.0 and are removed in Django 4.0
@jaap3
Copy link
Author

jaap3 commented Oct 19, 2021

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.

vstoykov added a commit to vstoykov/django-imagekit that referenced this pull request Nov 2, 2021
force_text and smart_text were deprecated in Django 3.0 and are removed in Django 4.0
@vstoykov vstoykov merged commit 6dd0a9a into matthewwithanm:develop Nov 2, 2021
@jaap3 jaap3 deleted the patch-1 branch November 3, 2021 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants