Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use "scale" mode as default. #66

Merged
merged 2 commits into from
Sep 29, 2022
Merged
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
2 changes: 0 additions & 2 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,8 @@ jobs:
matrix:
config:
# [Python version, tox env]
- ["3.6", "plone52-py36"]
- ["3.7", "plone52-py37"]
- ["3.8", "plone52-py38"]
- ["3.7", "plone60-py37"]
- ["3.8", "plone60-py38"]
- ["3.9", "plone60-py39"]
- ["3.10", "plone60-py310"]
Expand Down
6 changes: 6 additions & 0 deletions news/102.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Use "scale" mode as default.
This cleans up more confusion between mode and direction.
See also `plone.namedfile issue 102 <https://github.com/plone/plone.namedfile/issues/102>`_.
Previously our definition of the ``IImageScaleFactory`` interface had the deprecated ``direction="thumbnail"``.
Other parts used ``mode="contain"`` by default, which does cropping, where in Plone we are used to simple scaling almost everywhere.
[maurits]
2 changes: 2 additions & 0 deletions news/3637.breaking
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
No longer test Plone 5.2 on 3.6 and Plone 6.0 on 3.7.
[maurits]
10 changes: 6 additions & 4 deletions plone/scale/interfaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ class IScaledImageQuality(Interface):
class IImageScaleFactory(Interface):
"""Creates a scale"""

def _call__(
def __call__(
fieldname=None,
direction="thumbnail",
mode="scale",
height=None,
width=None,
scale=None,
Expand All @@ -30,8 +30,10 @@ def _call__(
``fieldname``
name of the field to scale

``direction``
is same as PIL direction on scale
``mode``
See ``scalePILImage`` for the values that should be accepted.
This used to be called "direction", which may still be accepted,
but is deprecated.

``width`` and ``height``
target size
Expand Down
22 changes: 11 additions & 11 deletions plone/scale/scale.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def scaleImage(
image,
width=None,
height=None,
mode="contain",
mode="scale",
quality=88,
result=None,
direction=None,
Expand Down Expand Up @@ -153,17 +153,17 @@ def _scale_thumbnail(image, width=None, height=None):
def get_scale_mode(mode, direction=None):
if direction is not None:
warnings.warn(
"the 'direction' option is deprecated, use 'mode' instead",
"The 'direction' option is deprecated, use 'mode' instead.",
DeprecationWarning,
)
mode = direction

if mode in ("scale-crop-to-fit", "down"):
mode = "contain"
elif mode in ("scale-crop-to-fill", "up"):
mode = "cover"
elif mode in ("keep", "thumbnail"):
mode = "scale"
if mode in ("scale", "keep", "thumbnail", None):
return "scale"
if mode in ("contain", "scale-crop-to-fit", "down"):
return "contain"
if mode in ("cover", "scale-crop-to-fill", "up"):
return "cover"

return mode

Expand All @@ -178,7 +178,7 @@ def __init__(self, original_width=0, original_height=0):


def _calculate_all_dimensions(
original_width, original_height, width, height, mode="contain"
original_width, original_height, width, height, mode="scale"
):
"""Calculate all dimensions we need for scaling.

Expand Down Expand Up @@ -334,7 +334,7 @@ def _calculate_all_dimensions(


def calculate_scaled_dimensions(
original_width, original_height, width, height, mode="contain"
original_width, original_height, width, height, mode="scale"
):
"""Calculate the scaled image dimensions from the originals using the
same logic as scalePILImage"""
Expand All @@ -345,7 +345,7 @@ def calculate_scaled_dimensions(
return (dimensions.final_width, dimensions.final_height)


def scalePILImage(image, width=None, height=None, mode="contain", direction=None):
def scalePILImage(image, width=None, height=None, mode="scale", direction=None):
"""Scale a PIL image to another size.

This is all about scaling for the display in a web browser.
Expand Down
3 changes: 2 additions & 1 deletion plone/scale/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,8 @@ def pre_scale(self, **parameters):
height = parameters.get("height")
orig_width, orig_height = value.getImageSize()
mode = get_scale_mode(
parameters.get("direction") or parameters.get("mode") or "contain"
parameters.get("mode"),
parameters.get("direction")
)
width, height = calculate_scaled_dimensions(
orig_width, orig_height, width, height, mode
Expand Down
13 changes: 8 additions & 5 deletions plone/scale/tests/test_scale.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ def testAlternativeSpellings(self):
``direction`` arguments instead of ``mode``.
"""

# Test mode contain. This can do cropping.
# scale-crop-to-fit
img = PIL.Image.new("RGB", (20, 20), (0, 0, 0))
img_scaled = scalePILImage(img, 10, 5, direction="scale-crop-to-fit")
Expand Down Expand Up @@ -335,19 +336,19 @@ def testDeprecations(self):
scaleImage(PNG, 16, 16, direction="keep")
self.assertEqual(len(w), 1)
self.assertIs(w[0].category, DeprecationWarning)
self.assertIn("the 'direction' option is deprecated", str(w[0].message))
self.assertIn("The 'direction' option is deprecated", str(w[0].message))

def test_calculate_scaled_dimensions_contain(self):
"""Test the calculate_scaled_dimensions function.
"""Test the calculate_scaled_dimensions function with mode "contain".

You pass it:

original_width, original_height, width, height

Plus an optional mode, by default "contain"`.
Plus an optional mode, by default "scale"`.
Alternative spellings: `scale-crop-to-fit`, `down`.
"""
calc = calculate_scaled_dimensions
calc = functools.partial(calculate_scaled_dimensions, mode="contain")
self.assertEqual(calc(1, 1, 1, 1), (1, 1))
self.assertEqual(calc(10, 10, 1, 1), (1, 1))
self.assertEqual(calc(1, 1, 10, 10), (10, 10))
Expand Down Expand Up @@ -381,8 +382,10 @@ def test_calculate_scaled_dimensions_scale(self):
"""Test calculate_scaled_dimensions function with mode "scale".

Alternative spellings: `keep`, `thumbnail`.
"scale" is the default
"""
calc = functools.partial(calculate_scaled_dimensions, mode="scale")
# calc = functools.partial(calculate_scaled_dimensions, mode="scale")
calc = calculate_scaled_dimensions
self.assertEqual(calc(1, 1, 1, 1), (1, 1))
self.assertEqual(calc(10, 10, 1, 1), (1, 1))
# Mode "scale" only scales down, not up:
Expand Down
15 changes: 13 additions & 2 deletions plone/scale/tests/test_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,9 @@ def testPreScaleForNonExistingScale(self):
self.assertIn("key", scale)
self.assertEqual(scale["data"], None)
self.assertEqual(scale["width"], 50)
self.assertEqual(scale["height"], 80)
# Note: the original image is 60x40.
# By default we scale without cropping, so you do not get height 80.
self.assertEqual(scale["height"], 33)
self.assertEqual(scale["mimetype"], "image/jpeg")
# Request the same pre scale.
scale2 = storage.pre_scale(width=50, height=80)
Expand All @@ -129,6 +131,15 @@ def testPreScaleForNonExistingScale(self):
self.assertEqual(new_scale["height"], 23)
self.assertEqual(new_scale["mimetype"], "image/png")

# Try cropping as well.
scale = storage.pre_scale(width=50, height=80, mode="contain")
self.assertIn("uid", scale)
self.assertIn("key", scale)
self.assertEqual(scale["data"], None)
self.assertEqual(scale["width"], 50)
self.assertEqual(scale["height"], 80)
self.assertEqual(scale["mimetype"], "image/jpeg")

def testPreScaleForNonExistingField(self):
self._provide_dummy_scale_adapter(None)
storage = self.storage
Expand All @@ -151,7 +162,7 @@ def test_get_or_generate(self):
self.assertEqual(placeholder["uid"], uid)
self.assertIsNone(placeholder["data"])
self.assertEqual(placeholder["width"], 50)
self.assertEqual(placeholder["height"], 80)
self.assertEqual(placeholder["height"], 33)
self.assertEqual(placeholder["mimetype"], "image/jpeg")
# 'get_or_generate' gets the pre generated placeholder info
# and generates the scale.
Expand Down
6 changes: 2 additions & 4 deletions tox.ini
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
[tox]
envlist =
plone52-py36,
plone52-py37,
plone52-py38,
plone60-py37,
plone60-py38,
plone60-py39,
plone60-py310,
Expand All @@ -18,13 +16,13 @@ commands =
pip list
zope-testrunner --test-path={toxinidir} {posargs:-vc}

[testenv:plone52-py{36,37,38}]
[testenv:plone52-py{37,38}]
commands_pre =
pip install mxdev
mxdev -c sources-52.ini
pip install -rrequirements-52-mxdev.txt

[testenv:plone60-py{37,38,39,310}]
[testenv:plone60-py{38,39,310}]
commands_pre =
# for libvcs pin, see https://github.com/bluedynamics/mxdev/issues/10
pip install mxdev "libvcs<0.12"
Expand Down