Skip to content

Conversation

jnovinger
Copy link
Member

This confirms support for Django 5.2 by modifying the test matrices, classifiers, and documentation alongside currently supported versions 4.2, 5.0, and 5.1.

I did not remove Django 5.0 support yet, even though it stopped being supported on April 2, 2025. I would be happy to add a commit to do so.

jnovinger and others added 2 commits October 6, 2025 11:55
This confirms support for Django 5.2 by modifying the test matrices,
classifiers, and documentation alongside currently supported versions
4.2, 5.0, and 5.1.
Copy link

codecov bot commented Oct 6, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.74%. Comparing base (0df1039) to head (c46636d).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #786      +/-   ##
==========================================
- Coverage   74.07%   69.74%   -4.34%     
==========================================
  Files          26       26              
  Lines        1678     1679       +1     
==========================================
- Hits         1243     1171      -72     
- Misses        435      508      +73     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jnovinger jnovinger requested a review from dulmandakh October 6, 2025 17:24
Copy link
Contributor

@dulmandakh dulmandakh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the CI

@claudep
Copy link
Contributor

claudep commented Oct 6, 2025

I guess we want to first find out why #781 fails.

@jnovinger
Copy link
Member Author

Hey, thanks all! I'm not super-familiar with the individual test that is failing in the [build (3.9, imagemagick)](https://github.com/jazzband/sorl-thumbnail/actions/runs/18288978456/job/52078701622?pr=786#logs) build:

  ======================================================================
  FAIL: test_orientation (tests.thumbnail_tests.test_templatetags.TemplateTestCaseA)
  ----------------------------------------------------------------------
  Traceback (most recent call last):
    File "/home/runner/work/sorl-thumbnail/sorl-thumbnail/tests/thumbnail_tests/test_templatetags.py", line 107, in test_orientation
      self.assertLess(epsilon(left, im.getpixel((7, 14))), 10)
  AssertionError: 62 not less than 10

Any guidance you can provide?

I also can't reproduce this on my local (Mac OS 15.6.1 w/ ImageMagick v7.x, versus the v6.9.12.98 which seems to be being run in Ubuntu 24.04 in Github Actions). I'm working on trying to reproduce via a Docker image, but that is not going quickly.

So, from what I can tell, the test is failing on ImageMagick 6.9.12.98
(Ubuntu 24.04), but passing on ImageMagick 7.

The root cause appears to be that ImageMagick 6 has a bug where
`-colorspace RGB` corrupts grayscale pixel values during conversion.
In our test, grayscale value 85 becomes 23 when `-colorspace RGB` is
applied, even though the colorspace isn't actually changed (still
reports as Gray).

Testing showed:
- `convert input.jpg -auto-orient -scale 30x30!` -> pixel value 85
- `convert input.jpg -auto-orient -colorspace RGB -scale 30x30!` -> pixel value 23
- `convert input.jpg -auto-orient -colorspace sRGB -scale 30x30!` -> pixel value 85

According to [ImageMagick's porting guide](https://imagemagick.org/script/porting.php),
IM6 "does not have a true grayscale colorspace, and only fakes it with
linear RGB colorspace", which likely explains this behavior.

Since the test is about orientation handling, not colorspace conversion,
the fix appears to be (for ImageMagick) specifically: pass `colorspace='sRGB'`
instead in this specific test, which works correctly on both IM6 and IM7.
However, that might break tests for other backends. The alternative is to
change the `THUMBNAIL_COLORSPACE` setting for the ImageMagick tests only
and fix the resulting breakages from cache key changes.

I couldn't find an existing bug report for this to reference. I'm
guessing the answer is the same-ish for jazzband#781. I can provide a
`Dockerfile` to reproduce the behavior (even on a Mac OS host) if you'd
like.
@jnovinger
Copy link
Member Author

I just realized neither of approaches to fixing the CI failures is likely to work as described in the commit message of c46636d above, because the tests are shared across all backends but giving one backend a setting change that affect cache key values will now break tests in other backends. I think.

I would definitely welcome feedback here on how to handle this.

@jnovinger
Copy link
Member Author

jnovinger commented Oct 7, 2025

@dulmandakh, based on my commit message and comment above, as well as @claudep's point about #781, I think we can all agree this is a pre-existing issue to this PR.

Looking at the CI status for the second commit (which does fix some CI issues), you can see that only the ImageMagick related tests are failing. I'd be happy to remove my follow up attempt commit to fix the remaining CI issues and create a new bug issue for it.

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

Successfully merging this pull request may close these issues.

3 participants