Fixes #773: Accept a tuple as blur radius and ensure to call PIL blur filter with a tuple #774
Conversation
…PIL blur filter with a tuple
sorl/thumbnail/engines/pil_engine.py
Outdated
| xy = self.radius | ||
| if isinstance(xy, (int, float)): | ||
| xy = (xy, xy) | ||
| return image.gaussian_blur(xy) |
There was a problem hiding this comment.
What about importing GaussianBlur from PIL.ImageFilter instead of copy-pasting pillow code?
There was a problem hiding this comment.
The difference between those two classes was the base class, pillows GaussianBlur is a MultibandFilter, although the only occurance of ImageFilter.Filter in pil_engine.py is GaussianBlur. Maybe there was a reason for that?
There was a problem hiding this comment.
Interestingly, the ImageFilter is only imported when no ImportError happens, i.E. PIL is available. Would it make sense to change this and import the GaussianBlur from Pillow in this PR? Or is that a case for another refactoring PR?
There was a problem hiding this comment.
Feel free to test some refactoring in a separate PR.
There was a problem hiding this comment.
By the way, I'd love to see some test around this functionality (I think there are no currently).
There was a problem hiding this comment.
I added an engine test in the SimpleTestCase and changed the pil_engine to use pillows GaussianBlur filter. I ran tox locally, but only with python 3.12.
I noticed that the other engines don't support blur anyways (so i borrowed the pil_engine unittest skipif). But each library has support for some kind of blur. I'd be happy to contribute to this feature with docs and tests in another PR.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #774 +/- ##
==========================================
+ Coverage 73.79% 74.04% +0.25%
==========================================
Files 26 26
Lines 1675 1676 +1
==========================================
+ Hits 1236 1241 +5
+ Misses 439 435 -4 ☔ View full report in Codecov by Sentry. |
|
Thanks! |
This pr fixes #773. Pillow has changed the accepted type of the blur radius from int to tuple(int, int).