django-countries
django-countries copied to clipboard
SmileyChris
Support Django 5.0
Builds on #424, which should be merged first. Add support for Django 5.0.
Currently there are three test failures:
FAILED django_countries/tests/test_admin_filters.py::TestCountryFilter::test_choices - AssertionError: False != True
FAILED django_countries/tests/test_fields.py::TestModelForm::test_translated_choices - AssertionError: 'Afghanistan' != 'Afganio'
FAILED django_countries/tests/test_widgets.py::TestCountrySelectWidget::test_render - AssertionError: 0 != 1 : Found 0 instances of '<option value="AU">
The latter two cases are caused by Django’s changes to handling choices. It seems that part of this change means that Django iterates over the Countries class at field initialization, rather than lazily when required. This means that the translations are done once rather than on-demand.
One option is to instead use a no-argument callable, which will always be called later. I tried adding __call__ to Countries but that didn’t work, I suspect a wrapping function will be required.
I think we can stop using LazyTypedChoiceField / LazyTypedMultipleChoiceField and all related classes on Django 5.0, since a callable for choices is treated lazily by Django. But unfortunately there’s a bug with that laziness, which I’ve reported upstream: https://code.djangoproject.com/ticket/34899 .
The fix ( https://github.com/django/django/pull/17370 ) was merged and released in Django 5.0b1. I have updated the code to use the new callable choices support and made the Lazy* classes no-ops on Django 5.0+.
I also added a fix for the admin change:
The
django.contrib.admin.AllValuesFieldListFilter,ChoicesFieldListFilter,RelatedFieldListFilter, andRelatedOnlyFieldListFilteradmin filters now handle multi-valued query parameters.
Everything is passing now.
@SmileyChris can this please be merged and released? It is blocking upgrade to Django 5.0 (I am testing RC1 now). Thank you.
You can install my commit from the Git repo with this specification for Pip:
django-countries @ git+https://github.com/SmileyChris/django-countries@58f258402072a18756a15daa81325428382bd946
(or with the appropriate syntax for Poetry etc.)
That’s what I’ve done for testing my client’s project on Django 5.0 RC1.
Yes, that's what I'm doing (django_5.0 branch from your fork) and it's working so I think it would be good to issue the release before Django 5.0 is released and people start upgrading. Thanks you for the PR.
I did install from pip using the above command, but I still have the below issue in django 5. Am I missing something ? @adamchainz would you have any idea ? It seems we are quite a few to have this issue.
\django_countries\fields.py", line 22, in <module>
_entry_points = importlib.metadata.entry_points().get(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'EntryPoints' object has no attribute 'get'
Any new on when this is going to be released? Django 5 it out for a while and after I tried to upgrade, my admin pages got broken. Here is a corresponding Django ticket:
https://code.djangoproject.com/ticket/35046
Any chance this could be released? It's blocking our upgrade to Django 5.0. Thanks.
I did install from pip using the above command, but I still have the below issue in django 5. Am I missing something ? @adamchainz would you have any idea ? It seems we are quite a few to have this issue.
\django_countries\fields.py", line 22, in <module> _entry_points = importlib.metadata.entry_points().get( ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ AttributeError: 'EntryPoints' object has no attribute 'get'
It looks like @adamchainz' code works with Python < 3.10 perfectly. However, the code doesn't work with Python 3.12.
If I understand correctly, the importlib.metadata API has been changed since Python 3.10.
Other projects had the same issue, e.g. https://github.com/ewels/rich-click/issues/140.
I've modified Adam's code to make it work with Python 3.10 and above: https://github.com/JulianTenschert/django-countries/commit/f085ab1eb38dc5aece08092a03b083d00622cf94. This works well.
@SmileyChris Whats needed to move forward with this? How can we help?
Thanks
Thanks for your efforts, however I think the current situation is not sustainable in the longer term. If anyone has direct contacts with Chris personally or someone else at Lincoln Loop, it would be very nice to push a bit so as to unblock this package and put it in a co-maintainership situation (by Jazzband adoption or any other mean).
Hey everyone, I managed to get in contact with SmileyChris and he gave me contributor access to the repo. I'm currently trying to familiarize myself with the state of the project. @adamchainz Thank you for creating this PR and keeping it alive. What is the current state of the PR? Does it still rely on the Django 4.2 PR or have they been combined in the meantime? Does it also address the python 3.12 fixes?
From my side, this PR takes priority and after I will look into updating the test matrix to test all currently supported django and python combination instead of only the latest.
I did not combine the Django 4.2 PR into this one, but I did merge in the Python 3.12 fix. Thank you for taking over the project!
Sorry I've been so absent! I finally got things polished up -- I had to fix transifex not pulling in changes due to their api changes, and some tox updates for 4 but it's done, and I pushed 7.6!
One small follow-up fix is needed: https://github.com/SmileyChris/django-countries/pull/460
Without this check even a trivial use of CountryFilter will raise an exception:
File "/usr/local/lib/python3.12/site-packages/django_countries/filters.py", line 27, in choices
selected = force_str(lookup) in value
^^^^^^^^^^^^^^^^^^^^^^^^^^
Exception Type: TypeError at /admin/country_test/person/
Exception Value: argument of type 'NoneType' is not iterable
