Support for captioned and ordered images in galleries #3017 #3018

jmcp · 2018-04-11T19:15:24Z

For galleries: this change provides support for per-image captions
and for specifying an order to display images in.

Pull Request Checklist

[ X ] I’ve read the guidelines for contributing.
[ X ] I updated AUTHORS.txt and CHANGES.txt (if the change is non-trivial) and documentation (if applicable).
[ X ] I tested my changes.

Description

Implements issue #3017.

Kwpolska · 2018-04-11T19:24:58Z

nikola/plugins/task/galleries.py

+       
+        Example:
+        
+        header.jpg This is the header image for my blog


This format won’t cut it. Filenames could contain spaces. Instead of a custom parser, use something like JSON or YAML.

For galleries: this change provides support for per-image captions and for specifying an order to display images in. We look for a file called metadata.yml in the gallery directory. If this file exists, we read 'name', 'caption' and 'order' from it and apply that metadata when generating the gallery. An entry must have at least 'name' to be considered valid. We cannot guarantee that yaml.safe_load_all() will return the elements in the same order that they are listed in the file.

For galleries: this change provides support for per-image captions and for specifying an order to display images in. We look for a file called metadata.yml in the gallery directory. If this file exists, we read 'name', 'caption' and 'order' from it and apply that metadata when generating the gallery. An entry must have at least 'name' to be considered valid. We cannot guarantee that yaml.safe_load_all() will return the elements in the same order that they are listed in the file. The user is warned about incorrectly formed YAML entries in metadata.yml (no 'name' found), and images found in the gallery directory which have no corresponding entry in metadata.yml. These images are appended to the end of the image list with the real filename as the caption.

Kwpolska · 2018-04-12T11:31:59Z

It looks like your commit was misattributed. Please add the e-mail address you used for this commit (jmcp your domain) to your GitHub profile so your commits will appear as yours.
You may also want to add yourself to AUTHORS.txt.

Kwpolska

A few more things to fix.

Kwpolska · 2018-04-12T11:26:21Z

docs/manual.rst

+Images in galleries may be provided with captions and given a specific
+ordering, by creating a file in the gallery directory called metadata.yml.
+This YAML file should contain a 'name' field for each image in the gallery,
+and (if desired) either or both of 'caption' and 'order'. 


This needs a specific example for multiple files and cases.

Kwpolska · 2018-04-12T11:26:48Z

nikola/conf.py.in

+# named metadata.yml, is YAML and must contain at least a 'name' field. Other
+# recognised field names are 'order' and 'caption'.
+# Defaults to True
+# GALLERY_SORT_BY_META_FILE = True


This doesn’t need to be a setting. If user provides a metadata.yml file and it contains order fields, try to use them. Otherwise, don’t. I can’t think of a legitimate use case that would have a metadata file with order fields and not want to use it.

Kwpolska · 2018-04-12T11:27:27Z

nikola/plugins/task/galleries.py

+                            img_titles.append(self.captions[fn])
+                        else:
+                            img_titles.append(fn)
+                            self.logger.warn(


No need for a warning IMO.

Made this a self.logger.debug() call instead.

Kwpolska · 2018-04-12T11:28:10Z

nikola/plugins/task/galleries.py

+                    # final element, so skip it
+                    if not img:
+                        continue
+                    if 'name' in img.keys():


If that is a dict, you can use if 'name' in img: — and please do that in other imgkeys cases below

Kwpolska · 2018-04-12T11:28:35Z

nikola/plugins/task/galleries.py

@@ -568,7 +635,7 @@ def url_from_path(p):
        else:
            img_list, thumbs, img_titles = [], [], []

-        photo_array = []
+        pre_photo = {}


photo_info perhaps?

Seems reasonable; done.

Kwpolska · 2018-04-12T11:30:46Z

nikola/plugins/task/galleries.py

+                photo_array.append(pre_photo.pop(entry))
+            # Do we have any orphan entries from metadata.yml?
+            if len(pre_photo) > 0:
+                for entry in pre_photo:


I would sort entries here and below (in else) so the ordering does not change due to dict randomness (on Python ≤3.5)

I'm uncertain about this suggestion.

We cannot sort photo_info since it's a dict, and unless we've got an order (implicitly or explicitly) specified from metadata.yml, then having unsorted output matches the existing behaviour. I'd prefer to leave this as-is.

Kwpolska · 2018-04-12T11:34:10Z

nikola/plugins/task/galleries.py

@@ -229,7 +236,18 @@ def gen_tasks(self):

                image_name_list = [os.path.basename(p) for p in image_list]

-                if self.kw['use_filename_as_title']:
+                if self.captions:


Why is this a global variable? This should be handled in a gallery-specific way. Nikola can be run with threading, in which case breakage will occur.

Instead, return captions, order from find_metadata() and use that.

Also: how does this work for multilingual sites?

Now using order, captions = self.find_metadata(gallery) as suggested, and adding both as elements of context.

I haven't got a multilingual site to with, unfortunately. Looking through the code, though, I believe that the only place where non-en might get caught is in the call to json.dumps(). If we specify ensure_ascii=False that should do the trick.

jmcp

I believe that I've addressed all the changes requested.
This is my first github PR so I'm unsure if I have got everything figured out correctly in order for this PR to progress to integration - I would really appreciate feedback to let me know what I've missed, or what process steps I have mucked up. Thankyou!

Kwpolska · 2018-04-17T14:15:04Z

docs/manual.rst

+.. code:: yaml
+
+    ---
+    name: ready-for-the-acid-wash.jpg


Is this necessary? I would prefer something where users can specify only the files they want changed from the default.

I've updated the docs to make it plain that you only need to list images for which a specific caption and/or order is required.

Kwpolska

This looks pretty good! Localization support is very simple to add (I added instructions in comments). Other than that, there are a few minor style improvements to make.

PS. I still haven’t bothered to run and test this.

PPS. No, we really don’t require a lot of special effort from contributors. Authors/changelog and making sure it works is basically it.

Kwpolska · 2018-04-17T18:46:47Z

nikola/plugins/task/galleries.py

+                        else:
+                            img_titles.append(fn)
+                            self.logger.debug(
+                                "Image {0} found in gallery but not listed "


(Feel free to go past 79 characters per line, especially if adhering to the limit makes code look ugly. Keep it below 100 if you can, 120 max.)

Kwpolska · 2018-04-17T18:47:37Z

nikola/plugins/task/galleries.py

@@ -393,6 +411,51 @@ def create_galleries(self):
                'uptodate': [utils.config_changed(self.kw.copy(), 'nikola.plugins.task.galleries:mkdir')],
            }

+    def find_metadata(self, gallery):
+        """Search for a gallery metadata file. Returns list, dict if found."""


Please merge the docstring and the comment below it into one large, multiline docstring.

Kwpolska · 2018-04-17T18:49:55Z

nikola/plugins/task/galleries.py

+                        else:
+                            order.append(img['name'])
+                    else:
+                        self.logger.warn("no 'name:' for ({0}) in {1}".format(


I would argue this is error-and-exception worthy.

Kwpolska · 2018-04-17T18:55:41Z

nikola/plugins/task/galleries.py

@@ -220,6 +225,8 @@ def gen_tasks(self):
                    self.kw[k] = self.site.GLOBAL_CONTEXT[k](lang)

                context = {}
+                context['order'] = order


Let’s make this translatable, it won’t be hard.

create order, captions inside this loop, so you can

pass lang to find_metadata, which

will use it to get the correct language path (I’ll comment on it below)

Kwpolska · 2018-04-17T18:59:01Z

nikola/plugins/task/galleries.py

+        # use an empty string.
+        # Returns a list (ordering) and dict (captions).
+
+        meta_path = os.path.join(gallery, "metadata.yml")


base_meta_path = os.path.join(gallery, "metadata.yml") localized_meta_path = utils.get_translation_candidate(self.site.config, base_meta_path, lang)

And then (pseudocode):

if localized path exists: used_path = localized else if base path exists: used_path = base else: return [], {} # all the other logic here, with `used_path`

Kwpolska · 2018-04-17T18:59:24Z

nikola/plugins/task/galleries.py

+        if os.path.isfile(meta_path):
+            self.logger.debug("Using {0} for gallery {1}".format(
+                meta_path, gallery))
+            self.meta_path = meta_path


One more global reference to remove here.

It is now only necessary to specify an image filename in metadata.yml if you really want to have a specific caption or ordering for an image. Updated manual to make this clear. Update galleries.py to make it an error to provide a YAML entry which does not have a name: field.

Kwpolska · 2018-04-19T16:19:43Z

nikola/plugins/task/galleries.py

+                photo_array.append(photo_info.pop(entry))
+            # Do we have any orphan entries from metadata.yml, or
+            # are the files from the gallery not listed in metadata.yml?
+            if len(photo_info) > 0:


if photo_info: would be better spelling of that (I fixed it)

Kwpolska · 2018-04-19T16:22:11Z

Thanks for contributing!

You may still want to add the e-mail address you used for the first two commits to your GitHub profile so your commits will appear as yours.

Kwpolska requested changes Apr 11, 2018

View reviewed changes

getnikola deleted a comment Apr 12, 2018

jmcp force-pushed the captioned-images branch from a653aff to 232a81b Compare April 12, 2018 01:11

getnikola deleted a comment Apr 12, 2018

jmcp mentioned this pull request Apr 12, 2018

Support for captioned and ordered images in galleries #3017

Closed

Kwpolska requested changes Apr 12, 2018

View reviewed changes

jmcp and others added 2 commits April 17, 2018 13:35

Address further code review comments

2a08791

Merge branch 'master' into captioned-images

739c6f6

jmcp commented Apr 17, 2018

View reviewed changes

Kwpolska reviewed Apr 17, 2018

View reviewed changes

Kwpolska mentioned this pull request Apr 17, 2018

[WIP] images metadata #2837

Closed

Kwpolska and others added 6 commits April 18, 2018 18:02

Merge branch 'master' into captioned-images

da97054

Fix pydocstyle failure

eecc294

Note about metadata.yml localization [ci skip]

441ca0e

Undo incorrect change in galleries.py

f1d92bc

Use if photo_Info and sort keys

956f7f0

Kwpolska reviewed Apr 19, 2018

View reviewed changes

Kwpolska approved these changes Apr 19, 2018

View reviewed changes

Kwpolska merged commit 7db87dd into getnikola:master Apr 19, 2018

Kwpolska mentioned this pull request Apr 19, 2018

Image metadata support #1286

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for captioned and ordered images in galleries #3017 #3018

Support for captioned and ordered images in galleries #3017 #3018

jmcp commented Apr 11, 2018

Kwpolska Apr 11, 2018

Kwpolska commented Apr 12, 2018

Kwpolska left a comment

Kwpolska Apr 12, 2018

jmcp Apr 16, 2018

Kwpolska Apr 12, 2018

jmcp Apr 16, 2018

Kwpolska Apr 12, 2018

jmcp Apr 16, 2018

Kwpolska Apr 12, 2018

jmcp Apr 17, 2018

Kwpolska Apr 12, 2018

jmcp Apr 16, 2018

Kwpolska Apr 12, 2018

jmcp Apr 17, 2018

Kwpolska Apr 12, 2018

jmcp Apr 16, 2018

jmcp left a comment

Kwpolska Apr 17, 2018

jmcp Apr 19, 2018

Kwpolska left a comment

Kwpolska Apr 17, 2018

Kwpolska Apr 17, 2018

Kwpolska Apr 17, 2018

Kwpolska Apr 17, 2018

Kwpolska Apr 17, 2018

Kwpolska Apr 17, 2018

Kwpolska Apr 19, 2018

Kwpolska commented Apr 19, 2018

Support for captioned and ordered images in galleries #3017 #3018

Support for captioned and ordered images in galleries #3017 #3018

Conversation

jmcp commented Apr 11, 2018

Pull Request Checklist

Description

Choose a reason for hiding this comment

Kwpolska commented Apr 12, 2018

Kwpolska left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmcp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kwpolska left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kwpolska commented Apr 19, 2018