Merge pull request #131 from plone/petschki-mode-scaling-take-3

Use mode parameter instead of deprecated direction (take 3)

.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,2 @@
+Use ``mode`` parameter instead of deprecated ``direction`` and warn user about it.
+[petschki, 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/namedfile/adapters.py

@@ -121,7 +121,6 @@ def get_original_image_url(self, fieldname, width, height):
             fieldname,
             width=width,
             height=height,
-            direction="thumbnail",
             pre=True,
             include_srcset=False,
         )

plone/namedfile/scaling.py

@@ -37,6 +37,7 @@
 
 import functools
 import logging
+import warnings
 
 
 logger = logging.getLogger(__name__)
@@ -274,13 +275,19 @@ def update_parameters(self, **parameters):
                 parameters["quality"] = quality
         return parameters
 
-    def create_scale(self, data, direction, height, width, **parameters):
+    def create_scale(self, data, mode, height, width, **parameters):
+        if "direction" in parameters:
+            warnings.warn(
+                "The 'direction' option is deprecated, use 'mode' instead.",
+                DeprecationWarning,
+            )
+            mode = parameters.pop("direction")
         return scaleImage(
-            data, direction=direction, height=height, width=width, **parameters
+            data, mode=mode, height=height, width=width, **parameters
         )
 
     def handle_image(
-        self, orig_value, orig_data, direction, height, width, **parameters
+        self, orig_value, orig_data, mode, height, width, **parameters
     ):
         """Return a scaled image, its mimetype format, and width and height."""
         if getattr(orig_value, "contentType", "") == "image/svg+xml":
@@ -294,7 +301,7 @@ def handle_image(
             return result
         try:
             result = self.create_scale(
-                orig_data, direction=direction, height=height, width=width, **parameters
+                orig_data, mode=mode, height=height, width=width, **parameters
             )
         except (ConflictError, KeyboardInterrupt):
             raise
@@ -311,7 +318,7 @@ def handle_image(
     def __call__(
         self,
         fieldname=None,
-        direction="thumbnail",
+        mode="scale",
         height=None,
         width=None,
         scale=None,
@@ -335,6 +342,14 @@ def __call__(
             # as image value (the first argument).
             dummy, format_ = orig_value.contentType.split("/", 1)
             return None, format_, (orig_value._width, orig_value._height)
+        if "direction" in parameters:
+            warnings.warn(
+                "The 'direction' option is deprecated, use 'mode' instead.",
+                DeprecationWarning,
+            )
+            # We must get rid of this duplicate parameter, otherwise it ends up in
+            # hashes and it negates the next condition.
+            mode = parameters.pop("direction")
         if (
             not parameters
             and height
@@ -356,7 +371,7 @@ def __call__(
             del parameters["modified"]
         try:
             result = self.handle_image(
-                orig_value, orig_data, direction, height, width, **parameters
+                orig_value, orig_data, mode, height, width, **parameters
             )
         finally:
             # Make sure the file is closed to avoid error:
@@ -448,9 +463,10 @@ def traverse(self, name, furtherPath):
     @deprecate("use property available_sizes instead")
     def getAvailableSizes(self, fieldname=None):
         if fieldname:
-            logger.warning(
+            warnings.warn(
                 "fieldname was passed to deprecated getAvailableSizes, but "
                 "will be ignored.",
+                DeprecationWarning,
             )
         return self.available_sizes
 
@@ -521,7 +537,7 @@ def scale(
         scale=None,
         height=None,
         width=None,
-        direction="thumbnail",
+        mode="scale",
         pre=False,
         include_srcset=None,
         **parameters,
@@ -536,9 +552,9 @@ def scale(
             fieldname = primary.fieldname
         if scale is not None:
             if width is not None or height is not None:
-                logger.warn(
-                    "A scale name and width/heigth are given. Those are"
-                    "mutually exclusive: solved by ignoring width/heigth and "
+                logger.warning(
+                    "A scale name and width/height are given. Those are "
+                    "mutually exclusive: solved by ignoring width/height and "
                     "taking name",
                 )
             available = self.available_sizes
@@ -557,7 +573,7 @@ def scale(
             fieldname=fieldname,
             height=height,
             width=width,
-            direction=direction,
+            mode=mode,
             scale=scale,
             **parameters,
         )
@@ -576,7 +592,7 @@ def scale(
                     fieldname=fieldname,
                     height=height,
                     width=width,
-                    direction=direction,
+                    mode=mode,
                     scale=scale,
                     storage=storage,
                     **parameters,
@@ -592,7 +608,7 @@ def calculate_srcset(
         scale=None,
         height=None,
         width=None,
-        direction="thumbnail",
+        mode="scale",
         storage=None,
         **parameters,
     ):
@@ -610,7 +626,7 @@ def calculate_srcset(
                 fieldname=fieldname,
                 height=height * hdScale["scale"] if height else height,
                 width=width * hdScale["scale"] if width else width,
-                direction=direction,
+                mode=mode,
                 **parameters,
             )
             if scale_src is None:
@@ -625,10 +641,10 @@ def tag(
         scale=None,
         height=None,
         width=None,
-        direction="thumbnail",
+        mode="scale",
         **kwargs,
     ):
-        scale = self.scale(fieldname, scale, height, width, direction, pre=True)
+        scale = self.scale(fieldname, scale, height, width, mode, pre=True)
         return scale.tag(**kwargs) if scale else None
 
     def picture(
@@ -733,7 +749,7 @@ def _tag_from_brain_image_scales(
         """Try to get a tag from the image_scales metadata.
 
         If we have any non-standard keyword arguments, we cannot use this method.
-        Especially you cannot set a direction: we must use the default "thumbnail".
+        Especially you cannot set a mode: we must use the default "scale" mode.
 
         Also, no old-style hidpi srcsets are included.  If the site has enabled this,
         we return nothing: this information is not (easily) available in the brain.

plone/namedfile/test.pt

@@ -69,7 +69,7 @@
   </section>
   <hr />
   <section id="examples">
-    <h2>Examples with direction/mode</h2>
+    <h2>Examples with mode</h2>
 
     <h3>Mini</h3>
     <figure class="figure"
@@ -78,16 +78,16 @@
       <br /><code tal:content="img_tag" />
     </figure>
 
-    <h3 id="cover">Mini direction=cover</h3>
+    <h3 id="cover">Mini mode=cover</h3>
     <figure class="figure"
-      tal:define="img_tag python:images.tag('image', scale='mini', direction='cover')">
+      tal:define="img_tag python:images.tag('image', scale='mini', mode='cover')">
       <img tal:replace="structure img_tag" />
       <br /><code tal:content="img_tag" />
     </figure>
 
-    <h3 id="contain">Mini direction=contain</h3>
+    <h3 id="contain">Mini mode=contain</h3>
     <figure class="figure"
-      tal:define="img_tag python:images.tag('image', scale='mini', direction='contain')">
+      tal:define="img_tag python:images.tag('image', scale='mini', mode='contain')">
       <img tal:replace="structure img_tag" />
       <br /><code tal:content="img_tag" />
     </figure>
@@ -96,9 +96,7 @@
   <section id="picture">
     <h2>Picture tags</h2>
     <p>
-      Temporary note:
-      Picture tags only work on Plone 6, with several other branches merged.
-      See <a href="https://github.com/plone/buildout.coredev/blob/6.0/plips/plip-image-srcsets.cfg">coredev</a>.
+      Picture tags only work on Plone 6.
       If not available (like on Plone 5.2), an ordinary image tag is created.
     </p>
 

plone/namedfile/tests/test_adapters.py

@@ -85,7 +85,7 @@ def test_field_adapter_return_scales(self):
             },
         )
         # Note: self.content.absolute_url() is actually empty in this test.
-        self.assertTrue(download.startswith(f"@@images/image1-16-"))
+        self.assertTrue(download.startswith("@@images/image1-16-"))
         self.assertTrue(download.endswith(".gif"))
         self.assertIn("listing", scales)
         self.assertEqual(len(scales), 1)
@@ -94,7 +94,7 @@ def test_field_adapter_return_scales(self):
         self.assertEqual(listing["height"], 16)
         self.assertEqual(listing["width"], 16)
         download = listing["download"]
-        self.assertTrue(download.startswith(f"@@images/image1-16-"))
+        self.assertTrue(download.startswith("@@images/image1-16-"))
         self.assertTrue(download.endswith(".gif"))
 
     @unittest.skipIf(IImageScalesFieldAdapter is not None, "Skipping on Plone 6")
@@ -131,7 +131,7 @@ def test_field_adapter_does_not_return_larger_scales(self):
             },
         )
         # Note: self.content.absolute_url() is actually empty in this test.
-        self.assertTrue(download.startswith(f"@@images/image1-900-"))
+        self.assertTrue(download.startswith("@@images/image1-900-"))
         self.assertTrue(download.endswith(".jpeg"))
         # larger and huge should not be in here: these scales would return the same
         # content as the original.
@@ -142,4 +142,4 @@ def test_field_adapter_does_not_return_larger_scales(self):
         preview = scales["preview"]
         self.assertEqual(preview["width"], 400)
         self.assertEqual(preview["height"], 400)
-        self.assertTrue(preview["download"].startswith(f"@@images/image1-400-"))
+        self.assertTrue(preview["download"].startswith("@@images/image1-400-"))

plone/namedfile/tests/test_scaling.py

@@ -30,6 +30,7 @@
 import re
 import time
 import unittest
+import warnings
 
 
 # Unique scale name used to be a uuid.uui4(),
@@ -672,7 +673,29 @@ def testGuardedAccess(self):
 
     def testGetAvailableSizes(self):
         self.scaling.available_sizes = {"foo": (60, 60)}
-        assert self.scaling.getAvailableSizes("image") == {"foo": (60, 60)}
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter("once")
+            self.assertEqual(
+                self.scaling.getAvailableSizes(),
+                {"foo": (60, 60)},
+            )
+            self.assertEqual(len(w), 1)
+            self.assertIs(w[0].category, DeprecationWarning)
+            self.assertIn(
+                "use property available_sizes instead",
+                str(w[0].message),
+            )
+            self.assertEqual(
+                self.scaling.getAvailableSizes("image"),
+                {"foo": (60, 60)},
+            )
+            self.assertEqual(len(w), 2)
+            self.assertIs(w[1].category, DeprecationWarning)
+            self.assertIn(
+                "fieldname was passed to deprecated getAvailableSizes, but "
+                "will be ignored.",
+                str(w[1].message),
+            )
 
     def testGetImageSize(self):
         assert self.scaling.getImageSize("image") == (200, 200)

plone/namedfile/usage.rst

@@ -357,25 +357,25 @@ several ways that you may reference scales from page templates.
 
 1. for full control you may do the tag generation explicitly::
 
-     <img tal:define="scales context/@@images;
-                      thumbnail python: scales.scale('image', width=64, height=64);"
+     <img tal:define="images context/@@images;
+                      thumbnail python: images.scale('image', width=64, height=64);"
           tal:condition="thumbnail"
           tal:attributes="src thumbnail/url;
                           width thumbnail/width;
                           height thumbnail/height" />
 
    This would create an up to 64 by 64 pixel scaled down version of the image
    stored in the "image" field.  It also allows for passing in additional
-   parameters support by `plone.scale`_'s ``scaleImage`` function, e.g.
-   ``direction`` or ``quality``.
+   parameters supported by the ``scaleImage`` function from ``plone.scale``,
+   e.g. ``mode`` or ``quality``.
 
-   .. _`plone.scale`: http://pypi.python.org/pypi/plone.scale
+   .. _`plone.scale`: https://pypi.org/project/plone.scale/
 
 2. for automatic tag generation with extra parameters you would use::
 
-     <img tal:define="scale context/@@images"
-          tal:replace="structure python: scale.scale('image',
-                       width=1200, height=800, direction='down').tag()" />
+     <img tal:define="images context/@@images"
+          tal:replace="structure python: images.tag('image',
+                       width=1200, height=800, mode='contain')" />
 
 3. It is possible to access scales via predefined named scale sizes, rather
    than hardcoding the dimensions every time you access a scale.  The scale
@@ -384,9 +384,8 @@ several ways that you may reference scales from page templates.
    scale name => (width, height).  A scale called 'mini' could then be accessed
    like this::
 
-     <img tal:define="scale context/@@images"
-          tal:replace="structure python: scale.scale('image',
-                       scale='mini').tag()" />
+     <img tal:define="images context/@@images"
+          tal:replace="structure python: images.tag('image', scale='mini')" />
 
    This would use the predefined scale size "mini" to determine the desired
    image dimensions, but still allow to pass in extra parameters.

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 --use-deprecated legacy-resolver -rrequirements-52-mxdev.txt
 
-[testenv:plone60-py{37,38,39,310}]
+[testenv:plone60-py{38,39,310}]
 commands_pre =
     pip install -U pip
     # for libvcs pin, see https://github.com/bluedynamics/mxdev/issues/10