jonasundderwolf / django-image-cropping

Django helper application to easily and non-destructively crop arbitrarily large images in admin and frontend.
Other
553 stars 130 forks source link

Django 4.x Support #176

Closed jwedel closed 2 years ago

jwedel commented 2 years ago

This PR fixes #173 and updates to Django 4.0.

Things that have been changed:

I've added a new tox env from Django 4.0 with Python 3.8 + 3.9 as previous versions are not supported anymore.

tox reports:

ERROR:  py36-django22: InterpreterNotFound: python3.6
ERROR:  py37-django22: InterpreterNotFound: python3.7
  py38-django22: commands succeeded
ERROR:   py39-django22: could not install deps [pillow==6.2.2, -r/Users/janwedel/Development/repos/django-image-cropping/example/requirements.txt, Django>=2.2,<3.0]; v = InvocationError("/Users/janwedel/Development/repos/django-image-cropping/.tox/py39-django22/bin/python -m pip install pillow==6.2.2 -r/Users/janwedel/Development/repos/django-image-cropping/example/requirements.txt 'Django>=2.2,<3.0'", 1)
ERROR:  py36-django30: InterpreterNotFound: python3.6
ERROR:  py37-django30: InterpreterNotFound: python3.7
  py38-django30: commands succeeded
ERROR:   py39-django30: could not install deps [pillow==6.2.2, -r/Users/janwedel/Development/repos/django-image-cropping/example/requirements.txt, Django>=3.0,<3.1]; v = InvocationError("/Users/janwedel/Development/repos/django-image-cropping/.tox/py39-django30/bin/python -m pip install pillow==6.2.2 -r/Users/janwedel/Development/repos/django-image-cropping/example/requirements.txt 'Django>=3.0,<3.1'", 1)
ERROR:  py36-django31: InterpreterNotFound: python3.6
ERROR:  py37-django31: InterpreterNotFound: python3.7
  py38-django31: commands succeeded
ERROR:   py39-django31: could not install deps [pillow==6.2.2, -r/Users/janwedel/Development/repos/django-image-cropping/example/requirements.txt, Django>=3.1,<3.2]; v = InvocationError("/Users/janwedel/Development/repos/django-image-cropping/.tox/py39-django31/bin/python -m pip install pillow==6.2.2 -r/Users/janwedel/Development/repos/django-image-cropping/example/requirements.txt 'Django>=3.1,<3.2'", 1)
ERROR:  py36-django32: InterpreterNotFound: python3.6
ERROR:  py37-django32: InterpreterNotFound: python3.7
  py38-django32: commands succeeded
ERROR:   py39-django32: could not install deps [pillow==6.2.2, -r/Users/janwedel/Development/repos/django-image-cropping/example/requirements.txt, Django>=3.2,<4]; v = InvocationError("/Users/janwedel/Development/repos/django-image-cropping/.tox/py39-django32/bin/python -m pip install pillow==6.2.2 -r/Users/janwedel/Development/repos/django-image-cropping/example/requirements.txt 'Django>=3.2,<4'", 1)
  py38-django40: commands succeeded
ERROR:   py39-django40: could not install deps [pillow==6.2.2, -r/Users/janwedel/Development/repos/django-image-cropping/example/requirements.txt, Django>=4.0,<4.1]; v = InvocationError("/Users/janwedel/Development/repos/django-image-cropping/.tox/py39-django40/bin/python -m pip install pillow==6.2.2 -r/Users/janwedel/Development/repos/django-image-cropping/example/requirements.txt 'Django>=4.0,<4.1'", 1)

I don't have all python versions installed, but surprisingly, the tests actually work even on Django 2.2. Didn't expect that.

I also change the purest config as it was pointing to a non existing tests directory. Not sure if this was intentional or not.

anrie commented 2 years ago

Hey @jwedel, thanks for your efforts to make the project ready for 4.0.

It's okay that you don't have all Python versions locally installed, but we should make sure all the checks for all versions pass on CI which they currently don't: https://github.com/jonasundderwolf/django-image-cropping/runs/5033268821?check_suite_focus=true It would be great if you could fix this.

jwedel commented 2 years ago

@anrie As I mentioned already in my PR description, I ran tox and it was successful on Django 2.2.

The problem with the build was that the dependency coverage==6.3 is only available for Python >= 3.7.

ERROR: Could not find a version that satisfies the requirement coverage==6.3

The build was running on 3.6 and then aborted before running with 3.7 and higher.

So, maybe this release with Django 4 support is a good time to cut support at least for Python 3.6?

Please, could you let tox run until the end so we can see if it works on Python >= 3.7?

EDIT: I will just remove 3.6 from the tox config to see if it would work. If you really want to get it running with Python 3.6, we/I need to find out which dependencies do not work with Python 3.6. and then downgrade.

jwedel commented 2 years ago

@anrie So, in my latest commit, as mentioned above, I have removed:

anrie commented 2 years ago

Hey @jwedel,

the removal of the pillow dependency seems fine to me. easy-thumbnails seems to require a recent version of Pillow anyway these days.

I am a bit more hesitant when it comes to Python 3.6. I see the reasoning of the maintainers of coverage.py, that Python 3.6 has reached end-of-life. On the other hand Django 3.2 LTS is still around for a while and supports Python 3.6: https://docs.djangoproject.com/en/4.0/releases/3.2/#python-compatibility

And coverage is only used internally. Do you see any problem with staying on coverage.py 6.2? Otherwise i would prefer to keep the tests for 3.6 and only generate the coverage for recent versions, where coverage is supported.

jwedel commented 2 years ago

@anrie no, I just don’t know if other deps also need to be downgraded again. And I’m actually having trouble to install python 3.6 on my Mac, installation/build fails so I cannot really test it locally. So I have to rely on the CI here. Can you somehow enable that the builds run on a commit? This would make the feedback loop a bit shorter for me.

I am wondering why the build still builds with python 3.6 even though I have removed it from tox config.

But I will add it again and downgrade coverage.

Let’s see…

anrie commented 2 years ago

@jwedel I think the builds still run because of this.

And as the 3.6-Build fails, the other jobs are aborted too. So for testing on your branch you should be able to adjust the build strategy.

jwedel commented 2 years ago

@anrie

https://github.com/jonasundderwolf/django-image-cropping/blob/36bace0528adba54eeae1d937a29427d42a0b78c/.github/workflows/test_and_coverage.yml#L34-L47

anrie commented 2 years ago

Hey @jwedel,

I guess we are close. Alle checks pass, it's just that the tests for 3.6 don't run at the moment: https://github.com/jonasundderwolf/django-image-cropping/runs/5057844917?check_suite_focus=true

Looks like your final commit/merge removed 3.6 from the envlist in tox again: https://github.com/jonasundderwolf/django-image-cropping/pull/176/files

jwedel commented 2 years ago

@anrie Damnit, I used some git wizardry to revers parts of my commit and messed it up... Hopefully, it's better now ^^

anrie commented 2 years ago

@jwedel Looks better now :+1:

Thanks a lot for your contribution! I merge this PR now and issue a new release after I had the chance to look a the remaining issues (coverage, Python 3.10).