Fix: ImageField losing progressive JPEG flag#7
Fix: ImageField losing progressive JPEG flag#7barash1311 wants to merge 3 commits intostrollby:feat/asyncfrom
Conversation
…date tox env deps # Conflicts: # docs/requirements.txt # Conflicts: # pyproject.toml # uv.lock
|
Ready for review. Let me know if any changes are needed. |
| "readthedocs-sphinx-ext==2.2.5", | ||
| "sphinx==7.4.7", | ||
| "sphinx-rtd-theme==3.0.2", | ||
| ] |
There was a problem hiding this comment.
This change was unintentional from my local setup. I’m reverting it now so the PR only contains the intended changes. Thanks for flagging this, and sorry for the trouble.
| requires-python = ">=3.10" | ||
| dependencies = [ | ||
| "pymongo (>=4.14,<5.0)", | ||
| "pymongo (>=4.13,<5.0)", |
There was a problem hiding this comment.
That change wasn’t intentional from my side.
my local setup auto-touched pyproject.toml. I’ll revert this and keep the PR focused on the fix.
| [project] | ||
| name = "mongoengine" | ||
| version = "0.30.0" | ||
| version = "0.29.0" |
There was a problem hiding this comment.
That change wasn’t intentional from my side.
Sorry about that, I was setting up the project locally (environment and dependencies) and it looks like my tooling auto touched pyproject.toml .
| progressive = True | ||
| else: | ||
| progressive = False | ||
| progressive = kwargs.get("progressive") |
There was a problem hiding this comment.
Can you explain the logic and how it fixes the issue?
Ref: https://pillow.readthedocs.io/en/stable/handbook/image-file-formats.html#jpeg-saving
There was a problem hiding this comment.
The current logic mixes two different sources for progressive:
- img.info.get("progressive") : value coming from the original image metadata
- kwargs.get("progressive"): value which user explicitly passes to save()
Because of this, the final value of progressive ends up being inconsistent and confusing as it can change depending on metadata and then get overridden again.This matches what the todo comment warning about i.e unused/bad implementation
my change simplifies this and make the behavior predictable and explicit as:
- If the user passes progressive=True and the format is JPEG then we enable progressive
- otherwise we disable progressive
Instead of partially inheriting behavior from img.info the save behavior now follows only what the user asked for at save time. This removes the ambiguity the TODO is pointing out and avoids hidden behavior coming from the source image
This also aligns with Pillow’s docs: progressive is a save time option for JPEGs, not something that should be implicitly inherited unless we explicitly support that behavior
If the project prefers preserving the original image’s progressive flag, we can later support a "keep" style option (similar as quality="keep" in Pillow), but this change was mainly to clean up the TODO and make the current behavior correct and deterministic
2 participants
Description:
This PR fixes an issue in ImageField where progressive JPEGs lost their progressive flag when saved using put or put_from_file.
Now the flag is preserved so the image keeps its original format.
What I did:
Why:
I noticed that progressive JPEGs were being re-encoded as normal JPEGs, which made them a bit larger and removed the progressive loading.
This fix keeps the behavior consistent and avoids unnecessary changes to the image.
Notes:
Tested locally with both sync and async tests. Everything passed fine.