Submitting GIF support changes #428
Conversation
There was a problem hiding this comment.
Thanks @captmiddy!
In terms of tests, I think the minimum we need is a new test in webapp_t.py that requests a GIF image as its source – probably as a GIF and a JPEG. You’ll need to add a new test image.
Do people use animated GIFs with IIIF? Feels more like a v3 thing – but a test that an animated GIF can be requested as a non-animated format (e.g. JPEG, PNG) would also be useful.
It would be nice to test ImageInfo directly as well, but I don’t think we have unit tests for that.
loris/img_info.py
Outdated
| @@ -200,7 +200,7 @@ def from_image_file(self, formats=[], max_size_above_full=200): | |||
|
|
|||
| if self.src_format == 'jp2': | |||
| self._from_jp2(self.src_img_fp) | |||
| elif self.src_format in ('jpg','tif','png'): | |||
| elif self.src_format in ('gif','jpg','tif','png'): | |||
There was a problem hiding this comment.
nit: space after comma please
|
Hi all, I just wanted to add my voice in support of this PR. I would be interested in seeing this functionality merged, as we support GIF images for some users in our Loris installation. In the past we've patched our install as this PR demonstrates, but it would be better to have it in the upstream repository and officially supported as an option. I'd be happy to help out with the requested changes if needed. |
|
@arthurian great! |
|
That would be great, I don't have time to work this right now with a bunch of other project work. We have been using this patch on our own version of Loris for a long time now but we have other local patches we have implemented that we should also look into. |
|
@captmiddy Just a heads up that I sent a PR to your branch with the requested changes (captmiddy#2). |
Added tests for requesting a GIF image as its source.
|
I didn't rebase the webapp_t.py file before merging in this change. I honestly don't have time, but the changes that are off shouldn't be hard to merge. I don't have write to attempt a resolution anyway. The requested tests do appear to be in place now though. |
|
@alexwlchan I got the test passing. You want to take a look and squash-merge if it looks OK? |
This is a pull request with GIF only changes. Once I hear back what types of tests would be expected, I will commit tests to the test folder for the GIF support.