From cd62c72dcc743b5f7689e4e13fdf7db87fdc2e69 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Sun, 26 Oct 2014 12:04:03 +0000 Subject: [PATCH] Changed API for setting/getting focal points Previously, we used a property to do this. Heres the advantages of the new way: - The old way felt a bit like it was pretending to be a database field when it wasn't. The new way makes it easier for the developer to understand that this is just a setter/getter for 4 fields and not a field itself. - Code looks nicer - Easier to override in subclasses - More like Django user model --- .../images/feature_detection.rst | 4 +-- wagtail/wagtailimages/models.py | 27 ++++++++++--------- wagtail/wagtailimages/views/frontend.py | 2 +- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/docs/core_components/images/feature_detection.rst b/docs/core_components/images/feature_detection.rst index 0b3d03873..9a67e758f 100644 --- a/docs/core_components/images/feature_detection.rst +++ b/docs/core_components/images/feature_detection.rst @@ -83,6 +83,6 @@ You can manually run feature detection on all images by running the following co from wagtail.wagtailimages.models import Image for image in Image.objects.all(): - if image.focal_point is None: - image.focal_point = image.get_suggested_focal_point() + if not image.has_focal_point(): + image.set_focal_point(image.get_suggested_focal_point()) image.save() diff --git a/wagtail/wagtailimages/models.py b/wagtail/wagtailimages/models.py index b3905755d..8c148a889 100644 --- a/wagtail/wagtailimages/models.py +++ b/wagtail/wagtailimages/models.py @@ -73,8 +73,7 @@ class AbstractImage(models.Model, TagSearchable): def __str__(self): return self.title - @property - def focal_point(self): + def get_focal_point(self): if self.focal_point_x is not None and \ self.focal_point_y is not None and \ self.focal_point_width is not None and \ @@ -86,8 +85,10 @@ class AbstractImage(models.Model, TagSearchable): height=self.focal_point_height, ) - @focal_point.setter - def focal_point(self, focal_point): + def has_focal_point(self): + return self.get_focal_point() is not None + + def set_focal_point(self, focal_point): if focal_point is not None: self.focal_point_x = focal_point.x self.focal_point_y = focal_point.y @@ -134,10 +135,10 @@ class AbstractImage(models.Model, TagSearchable): filter, created = Filter.objects.get_or_create(spec=filter) try: - if self.focal_point: + if self.has_focal_point(): rendition = self.renditions.get( filter=filter, - focal_point_key=self.focal_point.get_key(), + focal_point_key=self.get_focal_point().get_key(), ) else: rendition = self.renditions.get( @@ -150,11 +151,11 @@ class AbstractImage(models.Model, TagSearchable): # If we have a backend attribute then pass it to process # image - else pass 'default' backend_name = getattr(self, 'backend', 'default') - generated_image = filter.process_image(file_field.file, backend_name=backend_name, focal_point=self.focal_point) + generated_image = filter.process_image(file_field.file, backend_name=backend_name, focal_point=self.get_focal_point()) # generate new filename derived from old one, inserting the filter spec and focal point key before the extension - if self.focal_point is not None: - focal_point_key = "focus-" + self.focal_point.get_key() + if self.has_focal_point(): + focal_point_key = "focus-" + self.get_focal_point().get_key() else: focal_point_key = "focus-none" @@ -165,10 +166,10 @@ class AbstractImage(models.Model, TagSearchable): output_filename = filename_without_extension + '.' + extension generated_image_file = File(generated_image, name=output_filename) - if self.focal_point: + if self.has_focal_point(): rendition, created = self.renditions.get_or_create( filter=filter, - focal_point_key=self.focal_point.get_key(), + focal_point_key=self.get_focal_point().get_key(), defaults={'file': generated_image_file} ) else: @@ -222,9 +223,9 @@ def image_feature_detection(sender, instance, **kwargs): raise ImproperlyConfigured("pyOpenCV could not be found.") # Make sure the image doesn't already have a focal point - if instance.focal_point is None: + if not instance.has_focal_point(): # Set the focal point - instance.focal_point = instance.get_suggested_focal_point() + instance.set_focal_point(instance.get_suggested_focal_point()) # Receive the pre_delete signal and delete the file associated with the model instance. diff --git a/wagtail/wagtailimages/views/frontend.py b/wagtail/wagtailimages/views/frontend.py index 39c0b4ac1..7b03d900e 100644 --- a/wagtail/wagtailimages/views/frontend.py +++ b/wagtail/wagtailimages/views/frontend.py @@ -15,6 +15,6 @@ def serve(request, signature, image_id, filter_spec): raise PermissionDenied try: - return Filter(spec=filter_spec).process_image(image.file.file, HttpResponse(content_type='image/jpeg'), focal_point=image.focal_point) + return Filter(spec=filter_spec).process_image(image.file.file, HttpResponse(content_type='image/jpeg'), focal_point=image.get_focal_point()) except Filter.InvalidFilterSpecError: return HttpResponse("Invalid filter spec: " + filter_spec, content_type='text/plain', status=400)