Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -267,3 +267,4 @@ that much better:
* Terence Honles (https://github.com/terencehonles)
* Sean Bermejo (https://github.com/seanbermejo)
* Juan Gutierrez (https://github.com/juannyg)
* Barash Sharma (https://github.com/barash1311)
1 change: 1 addition & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Changelog
Development
===========
- (Fill this out as you fix issues and develop your features).
- Fix: preserve and allow explicit control of progressive JPEGs in ImageField put/aput (#<PR_NUMBER>, @barashsharma)

Changes in 0.30.0
=================
Expand Down
16 changes: 4 additions & 12 deletions mongoengine/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -1811,17 +1811,13 @@ def put(self, file_obj, **kwargs):
raise ValidationError("Invalid image: %s" % e)

# Progressive JPEG
# TODO: fixme, at least unused, at worst bad implementation
progressive = img.info.get("progressive") or False

if (
kwargs.get("progressive")
kwargs.get("progressive") is not None
and isinstance(kwargs.get("progressive"), bool)
and img_format == "JPEG"
):
progressive = True
else:
progressive = False
progressive = kwargs.get("progressive")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The current logic mixes two different sources for progressive:

  1. img.info.get("progressive") : value coming from the original image metadata
  2. 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:

  1. If the user passes progressive=True and the format is JPEG then we enable progressive
  2. 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


if field.size and (
img.size[0] > field.size["width"] or img.size[1] > field.size["height"]
Expand Down Expand Up @@ -1875,17 +1871,13 @@ async def aput(self, file_obj, **kwargs):
raise ValidationError("Invalid image: %s" % e)

# Progressive JPEG
# TODO: fixme, at least unused, at worst bad implementation
progressive = img.info.get("progressive") or False

if (
kwargs.get("progressive")
kwargs.get("progressive") is not None
and isinstance(kwargs.get("progressive"), bool)
and img_format == "JPEG"
):
progressive = True
else:
progressive = False
progressive = kwargs.get("progressive")

if field.size and (
img.size[0] > field.size["width"] or img.size[1] > field.size["height"]
Expand Down
11 changes: 2 additions & 9 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[project]
name = "mongoengine"
version = "0.30.0"
version = "0.29.0"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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 .

description = "MongoEngine is a Python Object-Document Mapper for working with MongoDB."
authors = [
{ name = "Harry Marr", email = "harry.marr@gmail.com" }
Expand Down Expand Up @@ -33,21 +33,14 @@ classifiers = [
]
requires-python = ">=3.10"
dependencies = [
"pymongo (>=4.14,<5.0)",
"pymongo (>=4.13,<5.0)",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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.

]

[dependency-groups]
dev = [
"ruff (>=0.14)",
"pre-commit (>=4.5)"
]
docs = [
"docutils==0.21.2",
"jinja2==3.1.6",
"readthedocs-sphinx-ext==2.2.5",
"sphinx==7.4.7",
"sphinx-rtd-theme==3.0.2",
]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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.

test = [
"pytest (>=9.0)",
"pytest-asyncio (>=1.3)",
Expand Down
Loading