Merge pull request #66 from plone/maurits-direction-versus-mode

Use "scale" mode as default.

.github/workflows/tests.yml

@@ -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"]

news/102.bugfix

@@ -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]

news/3637.breaking

@@ -0,0 +1,2 @@
+No longer test Plone 5.2 on 3.6 and Plone 6.0 on 3.7.
+[maurits]

plone/scale/interfaces.py

@@ -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,
@@ -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

plone/scale/scale.py

@@ -45,7 +45,7 @@ def scaleImage(
     image,
     width=None,
     height=None,
-    mode="contain",
+    mode="scale",
     quality=88,
     result=None,
     direction=None,
@@ -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
 
@@ -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.
 
@@ -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"""
@@ -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.

plone/scale/storage.py

@@ -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

plone/scale/tests/test_scale.py

@@ -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")
@@ -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))
@@ -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:

plone/scale/tests/test_storage.py

@@ -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)
@@ -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
@@ -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.

tox.ini

@@ -1,9 +1,7 @@
 [tox]
 envlist =
-    plone52-py36,
     plone52-py37,
     plone52-py38,
-    plone60-py37,
     plone60-py38,
     plone60-py39,
     plone60-py310,
@@ -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"