Django

Code

Ticket #4115 (closed: wontfix)

Opened 1 year ago

Last modified 6 months ago

contrib.thumbnails

Reported by: SmileyChris Assigned to: nobody
Component: Contrib apps Version: SVN
Keywords: Cc: brosner@gmail.com, nick.lane.au@gmail.com, frankie@grimboy.co.uk, orestis@orestis.gr, paolo@php3.it, jshaffer2112@gmail.com, jesse.lovelace@gmail.com, stuff4riqui@gmail.com, donspauldingii@gmail.com, sebastien.fievet@gmail.com, scott@divisionbyzero.com.au
Triage Stage: Design decision needed Has patch: 1
Needs documentation: 0 Needs tests: 0
Patch needs improvement: 0

Description

The idea has been bantered about in the dev group, so I thought I'd put together something to start the ball rolling at least.

Attachments

contrib.thumbnails.patch (27.3 kB) - added by SmileyChris on 04/22/07 04:36:38.
First cut... no tests but read the documentation for the low-down
contrib.thumbnails.2.patch (20.7 kB) - added by SmileyChris on 05/17/07 18:56:51.
Next cut
contrib.thumbnails.3.patch (24.1 kB) - added by SmileyChris on 05/17/07 23:45:58.
With tests (mostly Chris Moffitt's work)
contrib.thumbnails.4.patch (23.5 kB) - added by SmileyChris on 05/20/07 17:03:25.
Tests now inside of thumbnails.contrib (it's the new way) and it'll ignore the tests if PIL import fails
contrib.thumbnails.5.patch (23.2 kB) - added by SmileyChris on 06/12/07 19:45:40.
working tests this time, and fixed crop method
contrib.thumbnails.6.patch (23.3 kB) - added by SmileyChris on 06/17/07 16:30:15.
Don't append .jpg unless we really need to.
contrib.thumbnails.7.patch (23.3 kB) - added by SmileyChris on 06/17/07 16:33:57.
Fixing a small bug if size is passed as a list (must be converted to a tuple so it can be used as string formatting arguments)
contrib.thumbnails.8.patch (25.6 kB) - added by chris.moffitt@gmail.com on 09/14/07 13:35:40.
Updated patch set
contrib.thumbnails.9.patch (21.0 kB) - added by SmileyChris on 09/17/07 00:16:06.
reverted to my latest proper tests and added unicode safety
contrib.thumbnails.10.patch (24.3 kB) - added by SmileyChris on 09/18/07 17:42:44.
contrib.thumbnails.11.patch (24.3 kB) - added by SmileyChris on 10/08/07 23:04:23.
make thumbnail.url use urlquote (+ some formatting tweaks)
contrib.thumbnails.12.patch (24.5 kB) - added by SmileyChris on 10/09/07 17:00:48.
Take 2 on quoting the thumbnail url (thanks for the critiques, esaj)
contrib.thumbnails.13.patch (23.7 kB) - added by SmileyChris on 10/30/07 04:52:48.
Fix accidentally duplicated thumbnail creation, and img_tag filter incorrectly using file instead of filename

Change History

04/22/07 04:36:38 changed by SmileyChris

  • attachment contrib.thumbnails.patch added.

First cut... no tests but read the documentation for the low-down

04/22/07 10:30:00 changed by bin

  • needs_better_patch changed.
  • needs_tests changed.
  • needs_docs changed.

Good initiative Chris.

Some time ago I gave a shot at creating a set of generic views to interface with PIL in a RESTful manner. Maybe it's a nice idea to consider adding generic views to your contrib.

Regards, bin

04/22/07 16:34:32 changed by Simon G. <dev@simon.net.nz>

  • stage changed from Unreviewed to Design decision needed.

04/23/07 10:44:41 changed by brosner <brosner@gmail.com>

  • cc set to brosner@gmail.com.

(in reply to: ↑ description ) 04/29/07 04:47:14 changed by Mitja

See ThumbNails for discussion.

05/06/07 19:29:35 changed by nick.lane.au@gmail.com

  • cc changed from brosner@gmail.com to brosner@gmail.com, nick.lane.au@gmail.com.

Good idea, and I like the implementation.

05/07/07 10:05:34 changed by frankie@grimboy.co.uk

  • cc changed from brosner@gmail.com, nick.lane.au@gmail.com to brosner@gmail.com, nick.lane.au@gmail.com, frankie@grimboy.co.uk.

+1 for inclusion

05/11/07 06:22:20 changed by orestis@orestis.gr

  • cc changed from brosner@gmail.com, nick.lane.au@gmail.com, frankie@grimboy.co.uk to brosner@gmail.com, nick.lane.au@gmail.com, frankie@grimboy.co.uk, orestis@orestis.gr.
  • needs_tests set to 1.

+1 on this, I'll begin testing it...

05/16/07 13:07:32 changed by jeff@jeffcroftcom

+1 fo' sho'. Good stuff!

05/16/07 20:52:56 changed by jeff@jeffcroft.com

One suggestion to make this more useful (at least to me): We've found at the Journal-World that the default PIL JPG compression results in really crappy, artifacted images. After a good deal of playing, we decided that 85 was the ideal mix of filesize and image quality for us. I'd suggest the save function the "quality=85" argument:

thumbnail.save(self.filename, "JPEG", quality=85)

Obviously this is personal preference, so maybe it should be a setting, instead. In any case, I've never heard of any designer that is satisfied with the quality of the default JPG compression.

Great work!

05/17/07 18:56:51 changed by SmileyChris

  • attachment contrib.thumbnails.2.patch added.

Next cut

05/17/07 18:59:22 changed by SmileyChris

Here's the next cut.

  • I've added in Jeff's suggestion of being able to provide the JPEG quality.
  • Following on from the group discussion, I added in a helper template tag which creates the <img /> tag with the correct width & height for you.

05/17/07 19:00:20 changed by SmileyChris

I meant a helper template filter. It's named img_tag.

(follow-up: ↓ 14 ) 05/17/07 20:22:20 changed by Chris Moffitt <chris.moffitt@gmail.com>

The size is going to be returned incorrectly if a thumbnail is created. Here's a simple function I created to cache and return the actual size.

I'd recommend calling this in the img_tag filter.

size_cache = get_cache('locmem:///') _SIZE_CACHE_TIMEOUT = 60 * 60 * 60 * 24 * 31 # 1 month

def get_actual_size(self):

""" When an image is scaled, the dimensions of the image may not equal the supplied. Example 240x240 -> 240x96 actual size depending on the aspect ratio. This uses the cache to store the actual size instead of calling it each time an image is displayed. Normal use case is for image tags. """ size = size_cache.get(self.filename) if size:

return size

try:

img = Image.open(self.filename) size_cache.set(self.filename, im.size, _SIZE_CACHE_TIMEOUT) return img.size

except IOError, msg:

raise ThumbnailInvalidImage?(msg)

actual_size = property(get_actual_size)

05/17/07 20:31:49 changed by Chris Moffitt <chris.moffitt@gmail.com>

Ooops. Let's try that last code set again.

This should be put into the Thumbnail object.

size_cache = get_cache('locmem:///')
_SIZE_CACHE_TIMEOUT = 60 * 60 * 60 * 24 * 31 # 1 month

    def get_actual_size(self):
        """
        When an image is scaled, the dimensions of the image may not equal
        the supplied.  Example 240x240 -> 240x96 actual size depending on the aspect
        ratio.  This uses the cache to store the actual size instead of calling it each
        time an image is displayed.  Normal use case is for image tags.
        """
        size = size_cache.get(self.filename)
        if size:
            return size
        
        try:
            img = Image.open(self.filename)
            size_cache.set(self.filename, im.size, _SIZE_CACHE_TIMEOUT)
            return img.size
        except IOError, msg:
            raise ThumbnailInvalidImage(msg)
    actual_size = property(get_actual_size)

(in reply to: ↑ 12 ) 05/17/07 21:13:04 changed by SmileyChris

Chris Moffitt said: The size is going to be returned incorrectly if a thumbnail is created.

I think you're missing where the size is coming from.

thumbnail.thumbnail.size will return the correct size because the thumbnail property gets the PIL image object internally.

You do raise the valid question whether we should cache this rather than require a fs operation, but it does work the way it is now.

05/17/07 23:45:58 changed by SmileyChris

  • attachment contrib.thumbnails.3.patch added.

With tests (mostly Chris Moffitt's work)

05/17/07 23:46:18 changed by SmileyChris

  • needs_tests deleted.

05/18/07 02:30:16 changed by Marc Fargas <telenieko@telenieko.com>

Taking a quick look on the patch if you do not have PIL installed the test will immediately fail (ImportError?), but PIL is an optional dependency for django so the test just just say "Skipping this test because there's no PIL installed".

Just an idea :)

05/18/07 16:38:18 changed by SmileyChris

Darn, yea I meant to do that and forgot.

05/20/07 17:03:25 changed by SmileyChris

  • attachment contrib.thumbnails.4.patch added.

Tests now inside of thumbnails.contrib (it's the new way) and it'll ignore the tests if PIL import fails

05/29/07 04:43:30 changed by David Danier <goliath.mailinglist@gmx.de>

crop() had some errors with non-quadratic sizes here. It worked if crop_ratio was <= 1, but failed with crop_ratio > 1. I tried to fix this, leading to this code:

def crop(thumbnail): 
    """ Crop the image down to the same ratio as `size` """ 
    img = thumbnail.original_image 
    size = thumbnail.size 
 
    if img.size[0] < size[0] or img.size[1] < size[1]: 
        raise ThumbnailTooSmall('Image must be at least %s wide and %s high' % size) 
 
    image_x, image_y = img.size 
 
    crop_ratio = size[0] / float(size[1]) 
    image_ratio = image_x / float(image_y) 
 
    if crop_ratio < image_ratio: 
        # x needs to shrink 
        top = 0 
        bottom = image_y 
        if crop_ratio <= 1:
            crop_width = int(image_y * crop_ratio)
        else:
            crop_width = int(image_y / crop_ratio)
        left = (image_x - crop_width) // 2 
        right = left + crop_width 
    else: 
        # y needs to shrink 
        left = 0 
        right = image_x 
        if crop_ratio <= 1:
            crop_height = int(image_x * crop_ratio)
        else:
            crop_height = int(image_x / crop_ratio)
        top = (image_y - crop_height) // 2 
        bottom = top + crop_height 
 
    img = img.crop((left, top, right, bottom)) 
    return img.resize(size, Image.ANTIALIAS) 

06/12/07 19:45:40 changed by SmileyChris

  • attachment contrib.thumbnails.5.patch added.

working tests this time, and fixed crop method

06/16/07 13:09:15 changed by michelts@gmail.com

Hi, I've found 2 little bugs ;)

If the image size is shorter than the expected one, thumbnail filter should raise a ThumbnailTooShort? exception (methods.py:L10), it try a variable interpolation with the 2 itens list variable "size", the size is a list, not a tuple, should be converted to a tuple elsewhere (maybe in the base.py:L18).

If the image isn't a JPEG, thumbnails try to append ".jpg" to the end of filename, the string don't have a "append" method, should use += syntax (see the base.py:L34).

I will send a patch fixing these errors but now I am having problems with svn :D Thanks

06/17/07 16:29:36 changed by SmileyChris

I don't see why you need to convert a list to a tuple - it'll still do the same thing.

Regarding the appending of the filename and checking for both .jpg and .jpeg (case insensitively), that makes sense. Patch coming...

06/17/07 16:30:15 changed by SmileyChris

  • attachment contrib.thumbnails.6.patch added.

Don't append .jpg unless we really need to.

06/17/07 16:33:57 changed by SmileyChris

  • attachment contrib.thumbnails.7.patch added.

Fixing a small bug if size is passed as a list (must be converted to a tuple so it can be used as string formatting arguments)

06/17/07 16:35:24 changed by SmileyChris

I see the tuple/list bug now, fixed in .7

07/05/07 02:48:46 changed by anonymous

  • cc changed from brosner@gmail.com, nick.lane.au@gmail.com, frankie@grimboy.co.uk, orestis@orestis.gr to brosner@gmail.com, nick.lane.au@gmail.com, frankie@grimboy.co.uk, orestis@orestis.gr, paolo@php3.it.

07/05/07 15:33:02 changed by John Shaffer <jshaffer2112@gmail.com>

  • cc changed from brosner@gmail.com, nick.lane.au@gmail.com, frankie@grimboy.co.uk, orestis@orestis.gr, paolo@php3.it to brosner@gmail.com, nick.lane.au@gmail.com, frankie@grimboy.co.uk, orestis@orestis.gr, paolo@php3.it, jshaffer2112@gmail.com.

07/11/07 16:35:18 changed by christophrauch@gmail.com

Please incorporate following fix. The error happened on my own application.

--- thumbnails.py.orig  2007-07-11 23:30:09.000000000 +0200
+++ thumbnails.py       2007-07-11 23:30:56.000000000 +0200
@@ -84,6 +84,9 @@
     except OSError:
         # Couldn't read the original file.
         return ''
+    except IOError:
+        # Looks like it wasn't a file at all.
+        return ''
     
     # Generate the thumbnail.
     try:

07/13/07 21:51:06 changed by anonymous

  • cc changed from brosner@gmail.com, nick.lane.au@gmail.com, frankie@grimboy.co.uk, orestis@orestis.gr, paolo@php3.it, jshaffer2112@gmail.com to brosner@gmail.com, nick.lane.au@gmail.com, frankie@grimboy.co.uk, orestis@orestis.gr, paolo@php3.it, jshaffer2112@gmail.com, jesse.lovelace@gmail.com.

07/28/07 07:14:15 changed by orphansandoligarchs@gmail.com

Another quick fix to make it possible to thumbnail images which aren't RGB.

--- base.py.orig 2007-07-28 13:06:58.000000000 +0100
+++ base.py     2007-07-28 13:10:17.000000000 +0100
@@ -53,6 +53,8 @@
                 original = Image.open(StringIO(data))
             except IOError, msg:
                 raise ThumbnailInvalidImage(msg)
+            if original.mode not in ("L", "RGB"):
+                original = original.convert("RGB")
             self._original_image = original
             thumbnail = self.method()
             self._thumbnail = thumbnail


09/14/07 08:43:13 changed by mtredinnick

Just a note that Fredrik Lundh felt the patch (with the previous comment's change) looked resaonable. So we aren't doing any PIL silliness here.

09/14/07 08:48:13 changed by Fredrik Lundh <fredrik@pythonware.com>

Following up myself here; I have two minor nits:

* orphansandoligarchs makes a good point here; converting to RGB in case it's neither grayscale or colour is a good thing * PIL uses lazy loading of image data, which means that the first method that actually uses the pixels may fail as well. It could be worth moving the conversion and the method call into the try/except IOError part.

09/14/07 08:51:29 changed by Fredrik Lundh <fredrik@pythonware.com>

Ahem. And one somewhat larger:

- The thumbnail method modifies the source image in place, which means that using the "scale" method messes up the original image.

09/14/07 13:35:02 changed by chris.moffitt@gmail.com

I have modified patch set 7 to include the recommended updates from Frederik. I also updated the tests so that they pass now given the current naming scheme.

I've also attached the sample image used by the tests.

09/14/07 13:35:40 changed by chris.moffitt@gmail.com

  • attachment contrib.thumbnails.8.patch added.

Updated patch set

09/15/07 07:12:16 changed by sorl

To make filename with non-ascii characters to work, I guess this is since the unicode branch merge, I changed return thumbnail to return unicode(thumbnail) in thumbnails.py.

09/15/07 07:37:13 changed by Fredrik Lundh <fredrik@pythonware.com>

You meant "force_unicode", right? Python's "unicode" constructor throws an exception if you run it on a non-ASCII and don't specify an encoding.

09/16/07 17:16:27 changed by ubernostrum

See also #5476.

09/16/07 23:04:46 changed by sorl

Fredrik: actuallty not, smart_unicode and force_unicode throws error here: 'ascii' codec can't encode character...

09/17/07 00:16:06 changed by SmileyChris

  • attachment contrib.thumbnails.9.patch added.

reverted to my latest proper tests and added unicode safety

09/17/07 11:53:01 changed by jesse.lovelace@gmail.com

This last patch doesn't have the templatetags dir.

09/18/07 17:42:44 changed by SmileyChris

  • attachment contrib.thumbnails.10.patch added.

10/08/07 19:16:40 changed by esaj

Image URLs need to be properly formatted using django.utils.http.urlquote, e.g. ' ' converted to '%20'.

10/08/07 23:04:23 changed by SmileyChris

  • attachment contrib.thumbnails.11.patch added.

make thumbnail.url use urlquote (+ some formatting tweaks)

10/09/07 06:04:25 changed by esaj

Hmm, the latest patch urlquotes the base URL too, which breaks if you have "http://blah.com/" as your MEDIA_URL. It should assume that MEDIA_URL is already urlquoted, and only urlquote the image filename.

10/09/07 17:00:48 changed by SmileyChris

  • attachment contrib.thumbnails.12.patch added.

Take 2 on quoting the thumbnail url (thanks for the critiques, esaj)

10/18/07 17:00:25 changed by anonymous

  • cc changed from brosner@gmail.com, nick.lane.au@gmail.com, frankie@grimboy.co.uk, orestis@orestis.gr, paolo@php3.it, jshaffer2112@gmail.com, jesse.lovelace@gmail.com to brosner@gmail.com, nick.lane.au@gmail.com, frankie@grimboy.co.uk, orestis@orestis.gr, paolo@php3.it, jshaffer2112@gmail.com, jesse.lovelace@gmail.com, stuff4riqui@gmail.com.

10/18/07 22:03:44 changed by donspaulding

  • cc changed from brosner@gmail.com, nick.lane.au@gmail.com, frankie@grimboy.co.uk, orestis@orestis.gr, paolo@php3.it, jshaffer2112@gmail.com, jesse.lovelace@gmail.com, stuff4riqui@gmail.com to brosner@gmail.com, nick.lane.au@gmail.com, frankie@grimboy.co.uk, orestis@orestis.gr, paolo@php3.it, jshaffer2112@gmail.com, jesse.lovelace@gmail.com, stuff4riqui@gmail.com, donspauldingii@gmail.com.

10/28/07 10:38:35 changed by anonymous

  • cc changed from brosner@gmail.com, nick.lane.au@gmail.com, frankie@grimboy.co.uk, orestis@orestis.gr, paolo@php3.it, jshaffer2112@gmail.com, jesse.lovelace@gmail.com, stuff4riqui@gmail.com, donspauldingii@gmail.com to brosner@gmail.com, nick.lane.au@gmail.com, frankie@grimboy.co.uk, orestis@orestis.gr, paolo@php3.it, jshaffer2112@gmail.com, jesse.lovelace@gmail.com, stuff4riqui@gmail.com, donspauldingii@gmail.com, sebastien.fievet@gmail.com.

10/29/07 19:38:54 changed by Eric B <ebartels@gmail.com>

The current patch seems to be generating the thumbnail one time too many times (line 84 thumbnail.py) and then again (line 89). The first call isn't wrapped in a try statement, so it fails with a ThumbnailTooSmall? exception if the image is smaller than the specified thumbnail size.

10/30/07 04:52:48 changed by SmileyChris

  • attachment contrib.thumbnails.13.patch added.

Fix accidentally duplicated thumbnail creation, and img_tag filter incorrectly using file instead of filename

10/30/07 04:53:27 changed by SmileyChris

Thanks Eric, all fixed.

11/04/07 07:37:17 changed by Scott Barr <scott@divisionbyzero.com.au>

  • cc changed from brosner@gmail.com, nick.lane.au@gmail.com, frankie@grimboy.co.uk, orestis@orestis.gr, paolo@php3.it, jshaffer2112@gmail.com, jesse.lovelace@gmail.com, stuff4riqui@gmail.com, donspauldingii@gmail.com, sebastien.fievet@gmail.com to brosner@gmail.com, nick.lane.au@gmail.com, frankie@grimboy.co.uk, orestis@orestis.gr, paolo@php3.it, jshaffer2112@gmail.com, jesse.lovelace@gmail.com, stuff4riqui@gmail.com, donspauldingii@gmail.com, sebastien.fievet@gmail.com, scott@divisionbyzero.com.au.

11/11/07 08:30:21 changed by Christoph Rauch <christoph.rauch@gmail.com>

A few thoughts:

If an original image is smaller than the requested thumbnail size, ThumnailTooSmall? is raised. This makes sense as upscaling the image would increase its size.

I propose that instead of raising ThumbnailTooSmall?, we return the original image instead and add a mechanism to use HTML to do the upscaling. This mechanism should be able to decide if is best to use either height or width so the aspect ratio gets preserved.

I think we could make a wrapper around the thumbnail and img_tag templatetags. Perhaps change the current thumbnail to be thumbnail_url instead?

Any comments?

11/14/07 19:54:02 changed by esaj

I just noticed that it seems to scale the image even if it already has the correct dimensions, which (in my case) may unnecessarily add artefacts to a jpeg image. Instead, the image should just be copied as-is if it already has the correct dimensions.

11/15/07 08:00:43 changed by smokey

@esaj:

Creating a hard- or symlink if possible would be the better solution (me thinks). Not really portable to win32 though.

11/24/07 04:12:26 changed by Kellen

If installing this patch on Debian, it should go in: /usr/share/python-support/python-django/django/contrib/thumbnails/

But the same directory structure should be made in: /var/lib/python-support/python2.4/django/contrib/thumbnails/

with symlinks to each of the .py files, as with the other django.contrib directories found there.

(follow-up: ↓ 49 ) 12/01/07 05:01:28 changed by Kellen

With regards to ThumbnailTooSmall?, if one is using the thumbnail template filters for general-purpose resizing, then returning nothing to the template causes it to fail really ungracefully. I agree that returning the original image (with proper size info) is much, much better than failing.

This could be achieved by just returning the original image data instead of resized data. I personally think it's okay to replicate the image on disk with the expected filename as this will actually be of a smaller filesize than a generated thumbnail. It would be better to just return a Thumbnail object with the relevant data for the original image. The first option is easier to implement:

# diff methods.py methods.py.orig
10c10,11
<         return img
---
>         raise ThumbnailTooSmall('Image should be at least %s wide or %s high' %
>                                 tuple(size))
21c22,23
<         return img
---
>         raise ThumbnailTooSmall('Image must be at least %s wide and %s high' %
>                                 tuple(size))
30c32,33
<         return img
---
>         raise ThumbnailTooSmall('Image must be at least %s wide and %s high' %
>                                 tuple(size))

(in reply to: ↑ 48 ) 12/01/07 05:47:47 changed by Kellen

patch version, sorry:

--- methods.py.orig     2007-12-01 10:55:46.000000000 +0000
+++ methods.py  2007-12-01 10:56:34.000000000 +0000
@@ -7,8 +7,7 @@
     img = thumbnail.original_image
     size = thumbnail.size
     if img.size[0] < size[0] and img.size[1] < size[1]:
-        raise ThumbnailTooSmall('Image should be at least %s wide or %s high' %
-                                tuple(size))
+        return img
     # .thumbnail() method modifies in-place, so create a copy.
     img = img.copy()
     img.thumbnail(size, Image.ANTIALIAS)
@@ -19,8 +18,7 @@
     img = thumbnail.original_image
     size = thumbnail.size
     if img.size[0] < size[0] or img.size[1] < size[1]:
-        raise ThumbnailTooSmall('Image must be at least %s wide and %s high' %
-                                tuple(size))
+        return img
     # Just use ImageOps.fit (added in PIL 1.1.3)
     return ImageOps.fit(img, size, method=Image.ANTIALIAS)

@@ -29,6 +27,5 @@
     img = thumbnail.original_image
     size = thumbnail.size
     if img.size[0] < size[0] or img.size[1] < size[1]:
-        raise ThumbnailTooSmall('Image must be at least %s wide and %s high' %
-                                tuple(size))
+        return img
     return img.resize(size, Image.ANTIALIAS)

12/02/07 14:30:48 changed by jacob

  • status changed from new to closed.
  • resolution set to wontfix.

I think this is one of those projects that ought to be taken over to Google Code (or whereever) to get some real-world use before we consider adding it to Django.

12/02/07 14:36:47 changed by SmileyChris

It is as of a few days ago... I wanted to finalise things with sorl before I wrote here but jacob has forced my hand :)

I've taken a role in the development team of sorl-thumbnail: http://code.google.com/p/sorl-thumbnail/

Currently I am working on a branch of it which will refactor quite a lot of the code, making it similar in functionality to this ticket. It is mostly complete, there are just a few issues to resolve, like should it be a tag or a filter.

But anyway, that is where my thumbnails attention is going to be pointed. I recommend those who have followed this ticket to pay attention to that project.


Add/Change #4115 (contrib.thumbnails)




Change Properties
Action