modlinltd / django-advanced-filters

Add advanced filtering abilities to Django admin
https://pypi.org/project/django-advanced-filters/
MIT License
771 stars 173 forks source link

Versions of install_requires packages #79

Closed predatell closed 6 years ago

predatell commented 6 years ago

What is the reason to use these requirements?

install_requires=[
    'django-braces==1.4.0',
    'simplejson==3.6.5',
],

These versions are very outdated. Why not to use for example django-braces>=1.4.0 or simplejson>=3.6.5,<4.0.0

asfaltboy commented 6 years ago

Hi @predatell , thank you for opening a ticket. Normally we freeze requirements to avoid unintentional changes to new installations of django-advanced-filters.

Can you please clarify if you're having any specific issues?

predatell commented 6 years ago

For example I have another app in my project that needs django-braces>=1.8.0 and another app with requirement of simplejson>=3.8.0. But if I install django-advanced-filters it also installs simplejson==3.6.5 and django-braces==1.4.0.

If all other apps will use install_requires with links to any ONE minor version of requirements we will have many problems on installation of many apps in one project.

As I understand install_requires is used by owners of other packages for restrictions if their package does not work with some versions of requirements

asfaltboy commented 6 years ago

Ok gotcha. We normally freeze requirements to avoid them changing under us without our "consent". Bumping requirements is thus done manually.

Regarding django-braces we can merge that PR which depends on 1.9 and we can freeze at that version. I would prefer to not automatically permit any future (potentially breaking version) unless needed.

Regarding simplejson, I have no problem of allowing newer versions (e.g simplejson>=3.6.5,<4, but we'd need to test it first. Pull requests are welcome

predatell commented 6 years ago

Again, example (I use example for explaining my opinion). I am using app django-advanced-filters with requirements(django-braces==1.4.0, simplejson==3.6.5) in my project. Except this app I also have other 10 apps, for example django-advanced-filters1 with requirements(django-braces==1.3.0, simplejson==3.7.5), django-advanced-filters2 (django-braces==1.10.0, simplejson==3.14.0)... So if every app will have such requirements with linked certain versions I can have many problems in my project...

Now the latest version of django-braces is 1.13.0 (your app works with it), now you use django-braces with version 1.4.0. So it means that your app can work with django-braces from 1.4.0 to 1.13.0. So why not to use django-braces>=1.4.0,<=1.13.0 ? Common practice is to add any breaking changes to the first number in version. So my proposal to use: django-braces>=1.4.0,<=2.0.0

predatell commented 6 years ago

It is not my thoughts. It is official docs:

https://packaging.python.org/discussions/install-requires-vs-requirements/

install_requires is a setuptools setup.py keyword that should be used to specify what a project minimally needs to run correctly.

Additionally, it’s best practice to indicate any known lower or upper bounds.

It is not considered best practice to use install_requires to pin dependencies to specific versions, or to specify sub-dependencies (i.e. dependencies of your dependencies). This is overly-restrictive, and prevents the user from gaining the benefit of dependency upgrades.

asfaltboy commented 6 years ago

Nice reference, thanks! Once again, I have no immediate rejection to the proposal, please submit a PR and let's see how the tests fare.

PetrDlouhy commented 6 years ago

@asfaltboy I also think, that sticking to one requirement in Django application is a bad practice. The version freeze should be done in individual projects, not in applications.