From 93abbd0caa035d2ff178cbef0720cd7dfcef20b0 Mon Sep 17 00:00:00 2001 From: Antony Lee Date: Sat, 28 Feb 2015 19:44:38 -0800 Subject: [PATCH] Restore legacy TIFF API. To have the old API that always returns tuples, and fractions as pairs, set the `legacy_api` attribute of the IFD to True. This should alleviate concerns about backwards compatibility. --- PIL/TiffImagePlugin.py | 52 ++++++++---- Tests/helper.py | 5 -- Tests/test_file_gimpgradient.py | 2 +- Tests/test_file_libtiff.py | 52 +++++------- Tests/test_file_tiff.py | 136 ++++++++++--------------------- Tests/test_file_tiff_metadata.py | 78 ++++++++++++------ 6 files changed, 157 insertions(+), 168 deletions(-) diff --git a/PIL/TiffImagePlugin.py b/PIL/TiffImagePlugin.py index 8c6cf60a9..24a401331 100644 --- a/PIL/TiffImagePlugin.py +++ b/PIL/TiffImagePlugin.py @@ -247,10 +247,8 @@ class ImageFileDirectory(collections.MutableMapping): * self._tagdata = {} Key: numerical tiff tag number Value: undecoded byte string from file - Tags will be found in either self._tags or self._tagdata, but not - both. The union of the two should contain all the tags from the Tiff - image file. External classes shouldn't reference these unless they're - really sure what they're doing. + Tags will be found in the private attributes self._tagdata, and in + self._tags once decoded. """ def __init__(self, ifh=b"II\052\0\0\0\0\0", prefix=None): @@ -275,9 +273,17 @@ class ImageFileDirectory(collections.MutableMapping): raise SyntaxError("not a TIFF IFD") self.reset() self.next, = self._unpack("L", ifh[4:]) + self._legacy_api = False prefix = property(lambda self: self._prefix) offset = property(lambda self: self._offset) + legacy_api = property(lambda self: self._legacy_api) + + @legacy_api.setter + def legacy_api(self, value): + if value != self._legacy_api: + self._tags.clear() + self._legacy_api = value def reset(self): self._tags = {} @@ -302,18 +308,18 @@ class ImageFileDirectory(collections.MutableMapping): for code, value in self.items()) def __len__(self): - return len(self._tagdata) + len(self._tags) + return len(set(self._tagdata) | set(self._tags)) def __getitem__(self, tag): - try: - return self._tags[tag] - except KeyError: # unpack on the fly + if tag not in self._tags: # unpack on the fly data = self._tagdata[tag] typ = self.tagtype[tag] size, handler = self._load_dispatch[typ] self[tag] = handler(self, data) # check type - del self._tagdata[tag] - return self[tag] + val = self._tags[tag] + if self.legacy_api and not isinstance(val, (tuple, bytes)): + val = val, + return val def __contains__(self, tag): return tag in self._tags or tag in self._tagdata @@ -355,6 +361,8 @@ class ImageFileDirectory(collections.MutableMapping): for value in values] values = tuple(info.cvt_enum(value) for value in values) if info.length == 1: + if self.legacy_api and self.tagtype[tag] in [5, 10]: + values = values, self._tags[tag], = values else: self._tags[tag] = values @@ -364,7 +372,7 @@ class ImageFileDirectory(collections.MutableMapping): self._tagdata.pop(tag, None) def __iter__(self): - return itertools.chain(list(self._tags), list(self._tagdata)) + return iter(set(self._tagdata) | set(self._tags)) def _unpack(self, fmt, data): return struct.unpack(self._endian + fmt, data) @@ -398,10 +406,19 @@ class ImageFileDirectory(collections.MutableMapping): b"".join(self._pack(fmt, value) for value in values)) list(map(_register_basic, - [(1, "B", "byte"), (3, "H", "short"), (4, "L", "long"), + [(3, "H", "short"), (4, "L", "long"), (6, "b", "signed byte"), (8, "h", "signed short"), (9, "l", "signed long"), (11, "f", "float"), (12, "d", "double")])) + @_register_loader(1, 1) # Basic type, except for the legacy API. + def load_byte(self, data): + return (data if self.legacy_api else + tuple(map(ord, data) if bytes is str else data)) + + @_register_writer(1) # Basic type, except for the legacy API. + def write_byte(self, data): + return data + @_register_loader(2, 1) def load_string(self, data): if data.endswith(b"\0"): @@ -418,7 +435,9 @@ class ImageFileDirectory(collections.MutableMapping): @_register_loader(5, 8) def load_rational(self, data): vals = self._unpack("{0}L".format(len(data) // 4), data) - return tuple(num / denom for num, denom in zip(vals[::2], vals[1::2])) + combine = lambda a, b: (a, b) if self.legacy_api else a / b + return tuple(combine(num, denom) + for num, denom in zip(vals[::2], vals[1::2])) @_register_writer(5) def write_rational(self, *values): @@ -436,7 +455,9 @@ class ImageFileDirectory(collections.MutableMapping): @_register_loader(10, 8) def load_signed_rational(self, data): vals = self._unpack("{0}l".format(len(data) // 4), data) - return tuple(num / denom for num, denom in zip(vals[::2], vals[1::2])) + combine = lambda a, b: (a, b) if self.legacy_api else a / b + return tuple(combine(num, denom) + for num, denom in zip(vals[::2], vals[1::2])) @_register_writer(10) def write_signed_rational(self, *values): @@ -1026,11 +1047,14 @@ def _save(im, fp, filename): # inspired by image-sig posting from Kevin Cazabon, kcazabon@home.com if hasattr(im, 'tag'): # preserve tags from original TIFF image file + orig_api = im.tag.legacy_api + im.tag.legacy_api = False for key in (RESOLUTION_UNIT, X_RESOLUTION, Y_RESOLUTION, IPTC_NAA_CHUNK, PHOTOSHOP_CHUNK, XMP): if key in im.tag: ifd[key] = im.tag[key] ifd.tagtype[key] = im.tag.tagtype.get(key, None) + im.tag.legacy_api = orig_api # preserve ICC profile (should also work when saving other formats # which support profiles as TIFF) -- 2008-06-06 Florian Hoech diff --git a/Tests/helper.py b/Tests/helper.py index 83d86b5d9..abb2fbd6d 100644 --- a/Tests/helper.py +++ b/Tests/helper.py @@ -43,11 +43,6 @@ class PillowTestCase(unittest.TestCase): else: print("=== orphaned temp file: %s" % path) - def assert_almost_equal(self, a, b, msg=None, eps=1e-6): - self.assertLess( - abs(a-b), eps, - msg or "got %r, expected %r" % (a, b)) - def assert_deep_equal(self, a, b, msg=None): try: self.assertEqual( diff --git a/Tests/test_file_gimpgradient.py b/Tests/test_file_gimpgradient.py index c54dca7c1..1653fb304 100644 --- a/Tests/test_file_gimpgradient.py +++ b/Tests/test_file_gimpgradient.py @@ -80,7 +80,7 @@ class TestImage(PillowTestCase): ret = GimpGradientFile.sphere_increasing(middle, pos) # Assert - self.assert_almost_equal(ret, 0.9682458365518543) + self.assertAlmostEqual(ret, 0.9682458365518543) def test_sphere_decreasing(self): # Arrange diff --git a/Tests/test_file_libtiff.py b/Tests/test_file_libtiff.py index 703ba3015..2d0f2b041 100644 --- a/Tests/test_file_libtiff.py +++ b/Tests/test_file_libtiff.py @@ -2,7 +2,11 @@ from __future__ import print_function from helper import unittest, PillowTestCase, hopper, py3 import io +<<<<<<< HEAD import logging +======= +import itertools +>>>>>>> Restore legacy TIFF API. import os from PIL import Image, TiffImagePlugin @@ -123,41 +127,27 @@ class TestFileLibTiff(LibTiffTestCase): def test_write_metadata(self): """ Test metadata writing through libtiff """ - img = Image.open('Tests/images/hopper_g4.tif') - f = self.tempfile('temp.tiff') + for legacy_api in [False, True]: + img = Image.open('Tests/images/hopper_g4.tif') + img.tag.legacy_api = legacy_api + f = self.tempfile('temp.tiff') - img.save(f, tiffinfo=img.tag) + img.save(f, tiffinfo=img.tag) + original = img.tag.named() - loaded = Image.open(f) + # PhotometricInterpretation is set from SAVE_INFO, + # not the original image. + ignored = ['StripByteCounts', 'RowsPerStrip', 'PageNumber', + 'PhotometricInterpretation'] - original = img.tag.named() - reloaded = loaded.tag.named() + loaded = Image.open(f) + loaded.tag.legacy_api = legacy_api + reloaded = loaded.tag.named() - # PhotometricInterpretation is set from SAVE_INFO, - # not the original image. - ignored = [ - 'StripByteCounts', 'RowsPerStrip', - 'PageNumber', 'PhotometricInterpretation'] - - for tag, value in reloaded.items(): - if tag not in ignored: - if tag.endswith('Resolution'): - self.assert_almost_equal( - original[tag], value, - msg="%s didn't roundtrip" % tag) - else: - self.assertEqual( - original[tag], value, "%s didn't roundtrip" % tag) - - for tag, value in original.items(): - if tag not in ignored: - if tag.endswith('Resolution'): - self.assert_almost_equal( - original[tag], value, - msg="%s didn't roundtrip" % tag) - else: - self.assertEqual( - value, reloaded[tag], "%s didn't roundtrip" % tag) + for tag, value in itertools.chain(reloaded.items(), original.items()): + if tag not in ignored: + val = original[tag] + self.assertEqual(val, value, msg="%s didn't roundtrip" % tag) def test_g3_compression(self): i = Image.open('Tests/images/hopper_g4_500.tif') diff --git a/Tests/test_file_tiff.py b/Tests/test_file_tiff.py index 147507f54..4e3331058 100644 --- a/Tests/test_file_tiff.py +++ b/Tests/test_file_tiff.py @@ -73,12 +73,18 @@ class TestFileTiff(PillowTestCase): def test_xyres_tiff(self): from PIL.TiffImagePlugin import X_RESOLUTION, Y_RESOLUTION filename = "Tests/images/pil168.tif" - im = Image.open(filename) - # Try to read a file where X,Y_RESOLUTION are ints - im.tag[X_RESOLUTION] = (72,) - im.tag[Y_RESOLUTION] = (72,) - im._setup() - self.assertEqual(im.info['dpi'], (72., 72.)) + for legacy_api in [False, True]: + im = Image.open(filename) + im.tag.legacy_api = legacy_api + if legacy_api: + assert isinstance(im.tag[X_RESOLUTION][0], tuple) + assert isinstance(im.tag[Y_RESOLUTION][0], tuple) + # Try to read a file where X,Y_RESOLUTION are ints + im.tag[X_RESOLUTION] = (72,) + im.tag[Y_RESOLUTION] = (72,) + im.tag.legacy_api = False # _setup assumes the new API. + im._setup() + self.assertEqual(im.info['dpi'], (72., 72.)) def test_invalid_file(self): invalid_file = "Tests/images/flower.jpg" @@ -204,7 +210,6 @@ class TestFileTiff(PillowTestCase): self.assertEqual(im.convert('RGB').getpixel((0, 0)), (0, 0, 255)) def test___str__(self): - # Arrange filename = "Tests/images/pil136.tiff" im = Image.open(filename) @@ -217,127 +222,81 @@ class TestFileTiff(PillowTestCase): def test_as_dict(self): # Arrange filename = "Tests/images/pil136.tiff" - im = Image.open(filename) - - # Act - ret = im.ifd.as_dict() - - # Assert - self.assertIsInstance(ret, dict) - - self.assertEqual( - ret, {256: 55, 257: 43, 258: (8, 8, 8, 8), 259: 1, 262: 2, 296: 2, - 273: (8,), 338: (1,), 277: 4, 279: (9460,), - 282: 72.0, 283: 72.0, 284: 1}) + for legacy_api in [False, True]: + im = Image.open(filename) + im.tag.legacy_api = legacy_api + self.assertEqual( + im.tag.as_dict(), + {256: (55,), 257: (43,), 258: (8, 8, 8, 8), 259: (1,), + 262: (2,), 296: (2,), 273: (8,), 338: (1,), 277: (4,), + 279: (9460,), 282: ((720000, 10000),), + 283: ((720000, 10000),), 284: (1,)} if legacy_api else + {256: 55, 257: 43, 258: (8, 8, 8, 8), 259: 1, + 262: 2, 296: 2, 273: (8,), 338: (1,), 277: 4, + 279: (9460,), 282: 72.0, 283: 72.0, 284: 1}) def test__delitem__(self): - # Arrange filename = "Tests/images/pil136.tiff" im = Image.open(filename) len_before = len(im.ifd.as_dict()) - - # Act del im.ifd[256] - - # Assert len_after = len(im.ifd.as_dict()) self.assertEqual(len_before, len_after + 1) def test_load_byte(self): - # Arrange - ifd = TiffImagePlugin.ImageFileDirectory() - data = b"abc" - - # Act - ret = ifd.load_byte(data) - - # Assert - self.assertEqual(ret, (97, 98, 99)) + for legacy_api in [False, True]: + ifd = TiffImagePlugin.ImageFileDirectory() + ifd.legacy_api = legacy_api + data = b"abc" + ret = ifd.load_byte(data) + self.assertEqual(ret, b"abc" if legacy_api else (97, 98, 99)) def test_load_string(self): - # Arrange ifd = TiffImagePlugin.ImageFileDirectory() data = b"abc\0" - - # Act ret = ifd.load_string(data) - - # Assert self.assertEqual(ret, "abc") def test_load_float(self): - # Arrange ifd = TiffImagePlugin.ImageFileDirectory() data = b"abcdabcd" - - # Act ret = ifd.load_float(data) - - # Assert self.assertEqual(ret, (1.6777999408082104e+22, 1.6777999408082104e+22)) def test_load_double(self): - # Arrange ifd = TiffImagePlugin.ImageFileDirectory() data = b"abcdefghabcdefgh" - - # Act ret = ifd.load_double(data) - - # Assert self.assertEqual(ret, (8.540883223036124e+194, 8.540883223036124e+194)) def test_seek(self): - # Arrange filename = "Tests/images/pil136.tiff" im = Image.open(filename) - - # Act im.seek(-1) - - # Assert self.assertEqual(im.tell(), 0) def test_seek_eof(self): - # Arrange filename = "Tests/images/pil136.tiff" im = Image.open(filename) self.assertEqual(im.tell(), 0) - - # Act / Assert self.assertRaises(EOFError, lambda: im.seek(1)) def test__limit_rational_int(self): - # Arrange from PIL.TiffImagePlugin import _limit_rational value = 34 - - # Act ret = _limit_rational(value, 65536) - - # Assert self.assertEqual(ret, (34, 1)) def test__limit_rational_float(self): - # Arrange from PIL.TiffImagePlugin import _limit_rational value = 22.3 - - # Act ret = _limit_rational(value, 65536) - - # Assert self.assertEqual(ret, (223, 10)) def test_4bit(self): - # Arrange test_file = "Tests/images/hopper_gray_4bpp.tif" original = hopper("L") - - # Act im = Image.open(test_file) - - # Assert self.assertEqual(im.size, (128, 128)) self.assertEqual(im.mode, "L") self.assert_image_similar(im, original, 7.3) @@ -347,52 +306,45 @@ class TestFileTiff(PillowTestCase): # Test TIFF with tag 297 (Page Number) having value of 0 0. # The first number is the current page number. # The second is the total number of pages, zero means not available. - - # Arrange outfile = self.tempfile("temp.tif") - # Created by printing a page in Chrome to PDF, then: # /usr/bin/gs -q -sDEVICE=tiffg3 -sOutputFile=total-pages-zero.tif # -dNOPAUSE /tmp/test.pdf -c quit infile = "Tests/images/total-pages-zero.tif" im = Image.open(infile) - - # Act / Assert # Should not divide by zero im.save(outfile) def test_with_underscores(self): - # Arrange: use underscores kwargs = {'resolution_unit': 'inch', 'x_resolution': 72, 'y_resolution': 36} filename = self.tempfile("temp.tif") - - # Act hopper("RGB").save(filename, **kwargs) - - # Assert from PIL.TiffImagePlugin import X_RESOLUTION, Y_RESOLUTION - im = Image.open(filename) - self.assertEqual(im.tag[X_RESOLUTION], 72) - self.assertEqual(im.tag[Y_RESOLUTION], 36) + for legacy_api in [False, True]: + im = Image.open(filename) + im.tag.legacy_api = legacy_api + self.assertEqual(im.tag[X_RESOLUTION][0][0] if legacy_api + else im.tag[X_RESOLUTION], 72) + self.assertEqual(im.tag[Y_RESOLUTION][0][0] if legacy_api + else im.tag[Y_RESOLUTION], 36) def test_deprecation_warning_with_spaces(self): - # Arrange: use spaces kwargs = {'resolution unit': 'inch', 'x resolution': 36, 'y resolution': 72} filename = self.tempfile("temp.tif") - - # Act self.assert_warning(DeprecationWarning, lambda: hopper("RGB").save(filename, **kwargs)) - - # Assert from PIL.TiffImagePlugin import X_RESOLUTION, Y_RESOLUTION - im = Image.open(filename) - self.assertEqual(im.tag[X_RESOLUTION], 36) - self.assertEqual(im.tag[Y_RESOLUTION], 72) + for legacy_api in [False, True]: + im = Image.open(filename) + im.tag.legacy_api = legacy_api + self.assertEqual(im.tag[X_RESOLUTION][0][0] if legacy_api + else im.tag[X_RESOLUTION], 36) + self.assertEqual(im.tag[Y_RESOLUTION][0][0] if legacy_api + else im.tag[Y_RESOLUTION], 72) if __name__ == '__main__': diff --git a/Tests/test_file_tiff_metadata.py b/Tests/test_file_tiff_metadata.py index 9c7a451e3..df1934482 100644 --- a/Tests/test_file_tiff_metadata.py +++ b/Tests/test_file_tiff_metadata.py @@ -34,36 +34,64 @@ class TestFileTiffMetadata(PillowTestCase): img.save(f, tiffinfo=info) - loaded = Image.open(f) + for legacy_api in [False, True]: + loaded = Image.open(f) + loaded.tag.legacy_api = legacy_api + + self.assertEqual(loaded.tag[50838], + (len(basetextdata + " ?"),) if legacy_api else len(basetextdata + " ?")) + self.assertEqual(loaded.tag[50839], basetextdata + " ?") + loaded_float = loaded.tag[tag_ids['RollAngle']] + if legacy_api: + loaded_float = loaded_float[0] + self.assertAlmostEqual(loaded_float, floatdata, places=5) + loaded_double = loaded.tag[tag_ids['YawAngle']] + if legacy_api: + loaded_double = loaded_double[0] + self.assertAlmostEqual(loaded_double, doubledata) - self.assertEqual(loaded.tag[50838], (len(basetextdata + " ?"),)) - self.assertEqual(loaded.tag[50839], basetextdata + " ?") - self.assertAlmostEqual(loaded.tag[tag_ids['RollAngle']][0], floatdata, - places=5) - self.assertAlmostEqual(loaded.tag[tag_ids['YawAngle']], doubledata) def test_read_metadata(self): - img = Image.open('Tests/images/hopper_g4.tif') + for legacy_api in [False, True]: + img = Image.open('Tests/images/hopper_g4.tif') + img.tag.legacy_api = legacy_api - known = {'YResolution': 4294967295 / 113653537, - 'PlanarConfiguration': 1, - 'BitsPerSample': (1,), - 'ImageLength': 128, - 'Compression': 4, - 'FillOrder': 1, - 'RowsPerStrip': 128, - 'ResolutionUnit': 3, - 'PhotometricInterpretation': 0, - 'PageNumber': (0, 1), - 'XResolution': 4294967295 / 113653537, - 'ImageWidth': 128, - 'Orientation': 1, - 'StripByteCounts': (1968,), - 'SamplesPerPixel': 1, - 'StripOffsets': (8,) - } + known = {'YResolution': ((4294967295, 113653537),), + 'PlanarConfiguration': (1,), + 'BitsPerSample': (1,), + 'ImageLength': (128,), + 'Compression': (4,), + 'FillOrder': (1,), + 'RowsPerStrip': (128,), + 'ResolutionUnit': (3,), + 'PhotometricInterpretation': (0,), + 'PageNumber': (0, 1), + 'XResolution': ((4294967295, 113653537),), + 'ImageWidth': (128,), + 'Orientation': (1,), + 'StripByteCounts': (1968,), + 'SamplesPerPixel': (1,), + 'StripOffsets': (8,) + } if legacy_api else { + 'YResolution': 4294967295 / 113653537, + 'PlanarConfiguration': 1, + 'BitsPerSample': (1,), + 'ImageLength': 128, + 'Compression': 4, + 'FillOrder': 1, + 'RowsPerStrip': 128, + 'ResolutionUnit': 3, + 'PhotometricInterpretation': 0, + 'PageNumber': (0, 1), + 'XResolution': 4294967295 / 113653537, + 'ImageWidth': 128, + 'Orientation': 1, + 'StripByteCounts': (1968,), + 'SamplesPerPixel': 1, + 'StripOffsets': (8,) + } - self.assertEqual(known, img.tag.named()) + self.assertEqual(known, img.tag.named()) def test_write_metadata(self): """ Test metadata writing through the python code """