-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create Thumbnail from IIIF #209
base: main
Are you sure you want to change the base?
Conversation
iiif_prezi3/helpers/add_thumbnail.py
Outdated
|
||
if 'sizes' in image_info: | ||
best_fit_size = min( | ||
image_info['sizes'], key=lambda size: abs(size["width"] - preferred_width) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good to talk through this (so i can understand how it works :-))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally get the closest image which is bigger than requested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've refactored this to be:
best_fit_size = max(
(size for size in image_info['sizes'] if size["width"] <= preferred_width),
key=lambda size: size["width"]
)
The max function takes a list of dicts that represents the sizes prop of an image response and finds the best dict where the width of the dict is less than or equal to the preferred width that is specified. Origninally, this used abs()
to find the closest dict, but I've changed this to max()
to make sure the width is never larger than the width
that is preferred. Since we're dealing with a list of dicts and not a list of insts, we need to be able to compare in the max function just the width of the dict but ultimately return the best dict based on our criteria. This is where the key
comes in. This tells the max function to only look at the width key when comparing each dictionary to preferred width.
There is arguably a more readable and understandable way to do this, but I'm struggling to come up with a better way.
) | ||
thumbnail_id = f"{url.replace('/info.json', '')}/full/{best_fit_size['width']},{best_fit_size['height']}/0/default.jpg" | ||
else: | ||
thumbnail_id = f"{url.replace('/info.json', '')}/full/full/0/default.jpg" if context == "http://iiif.io/api/image/2/context.json" else f"{url.replace('/info.json', '')}/full/max/0/default.jpg" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this instead generate a image closer to the size provided? Assuming its not a level0 image.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@glenrobson, I still need to refactor for this. Is it safe to say that what's being suggested is if no sizes property exists and the image server is not level 0, this should return a thumbnail request for the exact width. Is this right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if level0 then return max or full
else
use exact side requested (for v2 width/ and for v3 width/(height)) height optional for v3 level 1 and above.
If you need test images I have: https://iiif-test.github.io/test2/images/IMG_5949/info.json is v3 and the files are at: https://github.com/iiif-test/test2/tree/main/images/IMG_5949 This is a v2: https://iiif-test.github.io/test2/images/IMG_8669/info.json and the files are at: https://github.com/iiif-test/test2/tree/main/images/IMG_8669 |
Added both to tests. |
Before I work on this any more, I think this should be rethought. Much of what I did originally followed how I did things in Canopy. I'm now questioning if what I set out to do originally is even correct and if this should go in a different direction. Instead of what's here currently, should this instead:
|
iiif_prezi3/helpers/add_thumbnail.py
Outdated
|
||
if 'sizes' in image_info: | ||
best_fit_size = max( | ||
(size for size in image_info['sizes'] if size["width"] <= preferred_width), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should get a bigger image as this can be shrinked with css and not loose quality. If we go smaller css will enlarge it and it may get pixilated.
self.assertEqual(thumbnail.id, "https://fixtures.iiif.io/other/level0/Glen/photos/gottingen/full/252,189/0/default.jpg") | ||
|
||
# check thumbnail service | ||
self.assertEqual(thumbnail.service[0].profile, "level0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check if you ask for slightly smaller than a size it picks the closest bigger one.
|
||
Args: | ||
url (str): An HTTP URL which points at a IIIF Image response. | ||
preferred_width (int, optional): the preferred width of the thumbnail. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mention you will get the closest bigger image in sizes
What Does This Do
This adds a new helper on
AddThumbnail
calledcreate_thumbnail_from_iiif()
. This allows someone to generate a thumbnail from an image info response that is v2 or v3. It optionally takes an argumentpreferred_width
that will choose the best thumbnail if asizes
property exists in the info.json (defaults to 500). If nosizes
property exists, it just takes the full sized image asfull/full
for v2 orfull/max
for v3. This seems like the safest way to approach in case of a level 0 image service.Some of the patterns here are inspired by previous helpers, but I chose to add the method to the existing AddThumbnail class rather than create a brand new helper class from scratch. My thinking about this was to try and keep all thumbnail helpers together. I'd like to add another helper method that let's you create preview thumbnails for videos, so my thinking was to try and reduce the amount of classes, but if each helper method needs its own class, I can refactor this.