Skip to content
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

Set minimum Django version in setup.py #89

Closed
threehundred opened this issue May 11, 2016 · 6 comments
Closed

Set minimum Django version in setup.py #89

threehundred opened this issue May 11, 2016 · 6 comments

Comments

@threehundred
Copy link

Traceback (most recent call last):
File "", line 1, in
File "/usr/local/lib/python2.7/site-packages/rest_framework_filters/init.py", line 3, in
from .filterset import FilterSet
File "/usr/local/lib/python2.7/site-packages/rest_framework_filters/filterset.py", line 13, in
from django_filters import filterset
File "/usr/local/lib/python2.7/site-packages/django_filters/init.py", line 3, in
from .filterset import FilterSet
File "/usr/local/lib/python2.7/site-packages/django_filters/filterset.py", line 19, in
from .filters import (Filter, CharFilter, BooleanFilter, BaseInFilter, BaseRangeFilter,
File "/usr/local/lib/python2.7/site-packages/django_filters/filters.py", line 16, in
from .fields import (
File "/usr/local/lib/python2.7/site-packages/django_filters/fields.py", line 13, in
from .utils import handle_timezone
File "/usr/local/lib/python2.7/site-packages/django_filters/utils.py", line 4, in
from django.db.models.expressions import Expression
ImportError: cannot import name Expression

Just an FYI - I couldn't find any django version restrictions in your release notes.

Cheers.

@threehundred
Copy link
Author

Sorry went back through history - I see you've dropped support for 1.7, will close.

@philipn
Copy link
Owner

philipn commented May 11, 2016

Thanks @threehundred! We should probably restrict the Django version in our setup.py, so I'm re-opening this issue.

@philipn philipn reopened this May 11, 2016
@rpkilby
Copy link
Collaborator

rpkilby commented May 11, 2016

@philipn I'm kind of wondering what should be done here. A couple of thoughts:

  • If we want to specify a minimum Django version, should we or should django-filter do this? ie, we're an extension of django-filter, which is an extension of Django. For all intents and purposes, we have the same compatibility guarantees as django-filter (also notice that the actual error in the trace was thrown from django-filter).
  • There is some push to not specify Django in the requires section. See Remove Django itself from install_requires jazzband/django-model-utils#183. In short, it can cause some issues across multiple pip installs.
  • A lot of projects don't specify django in the requires. eg, DRF, django-filter

@philipn
Copy link
Owner

philipn commented May 11, 2016

Hmm. You're totally right that few apps seem to list a Django dependency. No strong opinion, but my 2c--

It seems like people should be pinning their version of Django in their project's requirements (and using something sweet like https://github.com/nvie/pip-tools, to boot!) If they did, they'd never have problems like in the linked ticket(s). In the linked ticket, @carljm says "[django] doesn't need any automatic dependency handling", but this kind of issue is exactly why the dependency would help people -- they'd find out there's a problem during installation rather than at run time.

I wonder what kind of problem is more common -- hitting an unknown error during an upgrade because a package hasn't been tested / support has dropped for your version of Django, or having Django automatically upgraded and not wanting that behavior.

@rpkilby rpkilby changed the title Latest release does not work with django 1.7.9 Set minimum Django version in setup.py May 12, 2016
@rpkilby
Copy link
Collaborator

rpkilby commented May 25, 2016

Thinking back,

  • Adding Django as a dependency creates a problem for development environments, not deployments. eg, you would pip install -U djangorestframework-filters to test out the latest changes, and then accidentally update Django itself. Installing from a requirements file should complain about conflicts.
  • drf-filters is more an extension of django-filter than it is an extension of Django. The django-filter dependency should be sufficient?
  • drf-filters at least supports all currently supported versions of Django. I think the real answer here is "Don't use an outdated and unsupported version of Django".

@rpkilby
Copy link
Collaborator

rpkilby commented Aug 2, 2016

@philipn - Are we agreed that django shouldn't be listed as a requirement? Closing this as it's an easy modification if you decide otherwise.

@rpkilby rpkilby closed this as completed Aug 2, 2016
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

No branches or pull requests

3 participants