carltongibson / django-filter

A generic system for filtering Django QuerySets based on user selections
https://django-filter.readthedocs.io/en/main/
Other
4.41k stars 766 forks source link

handle_timezone is giving RemovedInDjango50Warning on Django 4.2 #1580

Closed lewiscollard closed 1 year ago

lewiscollard commented 1 year ago

When using django-filter's DateRangeField with Django 4.2, this warning occurs:

django.utils.deprecation.RemovedInDjango50Warning: The is_dst argument to make_aware(), used by the Trunc() database functions and QuerySet.datetimes(), is deprecated as it has no effect with zoneinfo time zones.

Being pedantic, I turn all test warnings into errors, so this causes my tests to fail.

I am not sure if this check is doing the best possible thing:

def handle_timezone(value, is_dst=None):
    if settings.USE_TZ and timezone.is_naive(value):
        if django.VERSION < (5, 0):
            ...

zoneinfo datetimes were introduced in 4.0 and pytz was deprecated at the same time. There is a USE_DEPRECATED_PYTZ setting for this case. So I think the check should look something like this:

a) if you're using a pre-4x version of Django (like 3.2, still supported for another ~11 months) for which is_dst is meaningful, then use is_dst b) if you're using a 4.x series and making use of USE_DEPRECATED_PYTZ, then use is_dst c) otherwise, don't use is_dst

i,e, that last check should look like:

if django.VERSION < (4, 0) or (django.VERSION < (5, 0) and getattr(settings, 'USE_DEPRECATED_PYTZ', False)):

That way, those of us that are behaving by not relying on deprecated behaviour will not get deprecation warnings, and everyone else gets the same behaviour they did before.

I am happy to do a PR for this if this seems like a good idea to you.

Thank you for a great Python package!

carltongibson commented 1 year ago

Hey @lewiscollard That sounds about right.