Merge lp:~malept/pyexiv2/0.2-ReadMetadataTestCase into lp:pyexiv2/0.2.x

[Mark Lee]

[Mark Lee (malept)]

Please see bug 517298 for details.

[Olivier Tilloy (osomon)]

Thanks for the patch Mark! Overall it looks good to me.

I have one remark on the consistency of the checks: the original case tests the python type of the values of EXIF and IPTC tags. In the case you added, the check is on the tag's type, not the type of the corresponding python object.

Also, how about merging some selected XMP metadata from exiv2-bug540.jpg into smiley1.jpg using exiv2 itself, rather than doing the testing on two different files, and what's more on a very large number of XMP tags?

In fact I can see many ways to improve the original test case, but that's definitely out of the scope of your patch. If you can fix the check consistency issue, then I'll merge your branch, and take some time to further enhance the tests.

review: Needs Fixing (code)

[Mark Lee (malept)]

> Thanks for the patch Mark! Overall it looks good to me.

Thanks for reviewing it.

> I have one remark on the consistency of the checks: the original case tests
> the python type of the values of EXIF and IPTC tags. In the case you added,
> the check is on the tag's type, not the type of the corresponding python
> object.

So you want to check tag.type (e.g., 'Ascii' for some strings)? I presume that you don't want to check tag.__class__ anymore, because that seems to always be ExifTag.

> Also, how about merging some selected XMP metadata from exiv2-bug540.jpg into
> smiley1.jpg using exiv2 itself, rather than doing the testing on two different
> files, and what's more on a very large number of XMP tags?

I can do that. I only did it this current way because it was easy to extract all of the XMP tags and throw them into the testcase. I presume I should put one (or two) XMP tags per type into smiley1.jpg?

[Olivier Tilloy (osomon)]

> > I have one remark on the consistency of the checks: the original case tests
> > the python type of the values of EXIF and IPTC tags. In the case you added,
> > the check is on the tag's type, not the type of the corresponding python
> > object.
>
> So you want to check tag.type (e.g., 'Ascii' for some strings)? I presume that
> you don't want to check tag.__class__ anymore, because that seems to always be
> ExifTag.

No, for the sake of consistency, I would rather check tag.value.__class__, as is done for EXIF and IPTC tags.

> > Also, how about merging some selected XMP metadata from exiv2-bug540.jpg
> into
> > smiley1.jpg using exiv2 itself, rather than doing the testing on two
> different
> > files, and what's more on a very large number of XMP tags?
>
> I can do that. I only did it this current way because it was easy to extract
> all of the XMP tags and throw them into the testcase. I presume I should put
> one (or two) XMP tags per type into smiley1.jpg?

No need to do it in a first step, the patch is correct without. Let's first merge the patch and then enhance the test case in a second step.

[Mark Lee (malept)]

> > > I have one remark on the consistency of the checks: the original case
> tests
> > > the python type of the values of EXIF and IPTC tags. In the case you
> added,
> > > the check is on the tag's type, not the type of the corresponding python
> > > object.
> >
> > So you want to check tag.type (e.g., 'Ascii' for some strings)? I presume
> that
> > you don't want to check tag.__class__ anymore, because that seems to always
> be
> > ExifTag.
>
> No, for the sake of consistency, I would rather check tag.value.__class__, as
> is done for EXIF and IPTC tags.

OK, I think I confused myself. I'll change the XMP tests to use Python types instead of ``tag.type``s.

[lp:~malept/pyexiv2/0.2-ReadMetadataTestCase]

273. By Mark Lee on 2010-02-17

XMP: Check the tag value types, not the tag type.

[Olivier Tilloy (osomon)]

Thanks for your work Mark!
I added a few custom tweaks and merged in 0.2.

review: Approve

=== modified file 'test/ReadMetadataTestCase.py'
--- test/ReadMetadataTestCase.py	2010-01-05 08:28:34 +0000
+++ test/ReadMetadataTestCase.py	2010-02-17 01:36:19 +0000
@@ -39,17 +39,38 @@
     Test case on reading the metadata contained in a file.
     """
 
-    def checkTypeAndValue(self, tag, etype, evalue):
-        """
-        Check the type and the value of a metadata tag against expected values.
-
-        Keyword arguments:
-        tag -- the full name of the tag (eg. 'Exif.Image.DateTime')
-        etype -- the expected type of the tag value
-        evalue -- the expected value of the tag
-        """
-        self.assertEqual(tag.__class__, etype)
-        self.assertEqual(tag, evalue)
+    def checkEXIFTypeAndValue(self, tag, etype, evalue):
+        """
+        Check the type and the value of an EXIF tag against expected values.
+
+        Keyword arguments:
+        tag -- the full name of the tag (eg. 'Exif.Image.DateTime')
+        etype -- the expected type of the tag value
+        evalue -- the expected value of the tag
+        """
+        self.assertEqual(type(tag.value), etype)
+        self.assertEqual(tag.value, evalue)
+
+    def checkIPTCTypeAndValue(self, tag, etype, evalue):
+        """
+        Check the type and the value of an IPTC tag against expected values.
+
+        Keyword arguments:
+        tag -- the full name of the tag (eg. 'Exif.Image.DateTime')
+        etype -- the expected type of the tag value
+        evalue -- the expected value of the tag
+        """
+        if issubclass(etype, tuple):
+            self.assertEqual(list(tag.values), list(evalue))
+        else:
+            self.assertEqual(type(tag.values[0]), etype)
+            self.assertEqual(tag.values[0], evalue)
+
+    def assertCorrectFile(self, filename, md5sum):
+        """
+        Ensure that the filename and the MD5 checksum match up.
+        """
+        self.assert_(testutils.CheckFileSum(filename, md5sum))
 
     def testReadMetadata(self):
         """
@@ -58,11 +79,11 @@
         # Check that the reference file is not corrupted
         filename = os.path.join('data', 'smiley1.jpg')
         md5sum = 'c066958457c685853293058f9bf129c1'
-        self.assert_(testutils.CheckFileSum(filename, md5sum))
+        self.assertCorrectFile(filename, md5sum)
 
         # Read the image metadata
-        image = pyexiv2.Image(filename)
-        image.readMetadata()
+        image = pyexiv2.ImageMetadata(filename)
+        image.read()
 
         # Exhaustive tests on the values of EXIF metadata
         exifTags = [('Exif.Image.ImageDescription', str, 'Well it is a smiley that happens to be green'),
@@ -77,9 +98,9 @@
                     ('Exif.Photo.Flash', int, 80),
                     ('Exif.Photo.PixelXDimension', long, 167L),
                     ('Exif.Photo.PixelYDimension', long, 140L)]
-        self.assertEqual(image.exifKeys(), [tag[0] for tag in exifTags])
-        for tag in exifTags:
-            self.checkTypeAndValue(image[tag[0]], tag[1], tag[2])
+        self.assertEqual(image.exif_keys, [tag[0] for tag in exifTags])
+        for key, ktype, value in exifTags:
+            self.checkEXIFTypeAndValue(image[key], ktype, value)
 
         # Exhaustive tests on the values of IPTC metadata
         iptcTags = [('Iptc.Application2.Caption', str, 'yelimS green faced dude (iptc caption)'),
@@ -93,11 +114,123 @@
                     ('Iptc.Application2.Category', str, 'Things'),
                     ('Iptc.Application2.Keywords', tuple, ('Green', 'Smiley', 'Dude')),
                     ('Iptc.Application2.Copyright', str, '\xa9 2004 Nobody')]
-        self.assertEqual(image.iptcKeys(), [tag[0] for tag in iptcTags])
-        for tag in iptcTags:
-            self.checkTypeAndValue(image[tag[0]], tag[1], tag[2])
-
-        # Test on the JPEG comment
-        self.checkTypeAndValue(image.getComment(),
-            str, 'This is a jpeg comment, about the green smiley.')
-
+        self.assertEqual(image.iptc_keys, [tag[0] for tag in iptcTags])
+        for key, ktype, value in iptcTags:
+            self.checkIPTCTypeAndValue(image[key], ktype, value)
+
+    def testReadMetadataXMP(self):
+        filename = os.path.join('data', 'exiv2-bug540.jpg')
+        md5sum = '64d4b7eab1e78f1f6bfb3c966e99eef2'
+        self.assertCorrectFile(filename, md5sum)
+
+        # Read the image metadata
+        image = pyexiv2.ImageMetadata(filename)
+        image.read()
+
+        xmpTags = [('Xmp.dc.creator', list, [u'Ian Britton']),
+                   ('Xmp.dc.description', dict, {u'x-default': u'Communications'}),
+                   ('Xmp.dc.rights', dict, {u'x-default': u'ian Britton - FreeFoto.com'}),
+                   ('Xmp.dc.source', unicode, u'FreeFoto.com'),
+                   ('Xmp.dc.subject', list, [u'Communications']),
+                   ('Xmp.dc.title', dict, {u'x-default': u'Communications'}),
+                   ('Xmp.exif.ApertureValue',
+                    pyexiv2.utils.Rational,
+                    pyexiv2.utils.Rational(8, 1)),
+                   ('Xmp.exif.BrightnessValue',
+                    pyexiv2.utils.Rational,
+                    pyexiv2.utils.Rational(333, 1280)),
+                   ('Xmp.exif.ColorSpace', int, 1),
+                   ('Xmp.exif.DateTimeOriginal',
+                    datetime.datetime,
+                    datetime.datetime(2002, 7, 13, 15, 58, 28, tzinfo=pyexiv2.utils.FixedOffset())),
+                   ('Xmp.exif.ExifVersion', unicode, u'0200'),
+                   ('Xmp.exif.ExposureBiasValue',
+                    pyexiv2.utils.Rational,
+                    pyexiv2.utils.Rational(-13, 20)),
+                   ('Xmp.exif.ExposureProgram', int, 4),
+                   ('Xmp.exif.FNumber',
+                    pyexiv2.utils.Rational,
+                    pyexiv2.utils.Rational(3, 5)),
+                   ('Xmp.exif.FileSource', int, 0),
+                   ('Xmp.exif.FlashpixVersion', unicode, u'0100'),
+                   ('Xmp.exif.FocalLength',
+                    pyexiv2.utils.Rational,
+                    pyexiv2.utils.Rational(0, 1)),
+                   ('Xmp.exif.FocalPlaneResolutionUnit', int, 2),
+                   ('Xmp.exif.FocalPlaneXResolution',
+                    pyexiv2.utils.Rational,
+                    pyexiv2.utils.Rational(3085, 256)),
+                   ('Xmp.exif.FocalPlaneYResolution',
+                    pyexiv2.utils.Rational,
+                    pyexiv2.utils.Rational(3085, 256)),
+                   ('Xmp.exif.GPSLatitude',
+                    pyexiv2.utils.GPSCoordinate,
+                    pyexiv2.utils.GPSCoordinate.from_string('54,59.380000N')),
+                   ('Xmp.exif.GPSLongitude',
+                    pyexiv2.utils.GPSCoordinate,
+                    pyexiv2.utils.GPSCoordinate.from_string('1,54.850000W')),
+                   ('Xmp.exif.GPSMapDatum', unicode, u'WGS84'),
+                   ('Xmp.exif.GPSTimeStamp',
+                    datetime.datetime,
+                    datetime.datetime(2002, 7, 13, 14, 58, 24, tzinfo=pyexiv2.utils.FixedOffset())),
+                   ('Xmp.exif.GPSVersionID', unicode, u'2.0.0.0'),
+                   ('Xmp.exif.ISOSpeedRatings', list, [0]),
+                   ('Xmp.exif.MeteringMode', int, 5),
+                   ('Xmp.exif.PixelXDimension', int, 2400),
+                   ('Xmp.exif.PixelYDimension', int, 1600),
+                   ('Xmp.exif.SceneType', int, 0),
+                   ('Xmp.exif.SensingMethod', int, 2),
+                   ('Xmp.exif.ShutterSpeedValue',
+                    pyexiv2.utils.Rational,
+                    pyexiv2.utils.Rational(30827, 3245)),
+                   ('Xmp.pdf.Keywords', unicode, u'Communications'),
+                   ('Xmp.photoshop.AuthorsPosition', unicode, u'Photographer'),
+                   ('Xmp.photoshop.CaptionWriter', unicode, u'Ian Britton'),
+                   ('Xmp.photoshop.Category', unicode, u'BUS'),
+                   ('Xmp.photoshop.City', unicode, u' '),
+                   ('Xmp.photoshop.Country', unicode, u'Ubited Kingdom'),
+                   ('Xmp.photoshop.Credit', unicode, u'Ian Britton'),
+                   ('Xmp.photoshop.DateCreated', datetime.date, datetime.date(2002, 6, 20)),
+                   ('Xmp.photoshop.Headline', unicode, u'Communications'),
+                   ('Xmp.photoshop.State', unicode, u' '),
+                   ('Xmp.photoshop.SupplementalCategories', list, [u'Communications']),
+                   ('Xmp.photoshop.Urgency', int, 5),
+                   ('Xmp.tiff.Artist', unicode, u'Ian Britton'),
+                   ('Xmp.tiff.BitsPerSample', list, [8]),
+                   ('Xmp.tiff.Compression', int, 6),
+                   ('Xmp.tiff.Copyright',
+                    dict,
+                    {u'x-default': u'ian Britton - FreeFoto.com'}),
+                   ('Xmp.tiff.ImageDescription', dict, {u'x-default': u'Communications'}),
+                   ('Xmp.tiff.ImageLength', int, 400),
+                   ('Xmp.tiff.ImageWidth', int, 600),
+                   ('Xmp.tiff.Make', unicode, u'FUJIFILM'),
+                   ('Xmp.tiff.Model', unicode, u'FinePixS1Pro'),
+                   ('Xmp.tiff.Orientation', int, 1),
+                   ('Xmp.tiff.ResolutionUnit', int, 2),
+                   ('Xmp.tiff.Software', unicode, u'Adobe Photoshop 7.0'),
+                   ('Xmp.tiff.XResolution',
+                    pyexiv2.utils.Rational,
+                    pyexiv2.utils.Rational(300, 1)),
+                   ('Xmp.tiff.YCbCrPositioning', int, 2),
+                   ('Xmp.tiff.YResolution',
+                    pyexiv2.utils.Rational,
+                    pyexiv2.utils.Rational(300, 1)),
+                   ('Xmp.xmp.CreateDate',
+                    datetime.datetime,
+                    datetime.datetime(2002, 7, 13, 15, 58, 28, tzinfo=pyexiv2.utils.FixedOffset())),
+                   ('Xmp.xmp.ModifyDate',
+                    datetime.datetime,
+                    datetime.datetime(2002, 7, 19, 13, 28, 10, tzinfo=pyexiv2.utils.FixedOffset())),
+                   ('Xmp.xmpBJ.JobRef', list, []),
+                   ('Xmp.xmpBJ.JobRef[1]', str, ''),
+                   ('Xmp.xmpBJ.JobRef[1]/stJob:name', str, 'Photographer'),
+                   ('Xmp.xmpMM.DocumentID',
+                    str,
+                    'adobe:docid:photoshop:84d4dba8-9b11-11d6-895d-c4d063a70fb0'),
+                   ('Xmp.xmpRights.Marked', bool, True),
+                   ('Xmp.xmpRights.WebStatement', str, 'www.freefoto.com')]
+        for key, xtype, value in xmpTags:
+            tag = image[key]
+            self.assertEqual(type(tag.value), xtype)
+            self.assertEqual(tag.value, value)

=== modified file 'test/TestsRunner.py'
--- test/TestsRunner.py	2010-01-19 18:42:25 +0000
+++ test/TestsRunner.py	2010-02-17 01:36:19 +0000
@@ -28,7 +28,7 @@
 import unittest
 
 # Test cases to run
-#from ReadMetadataTestCase import ReadMetadataTestCase
+from ReadMetadataTestCase import ReadMetadataTestCase
 #from Bug146313_TestCase import Bug146313_TestCase
 #from Bug173387_TestCase import Bug173387_TestCase
 #from Bug175070_TestCase import Bug175070_TestCase
@@ -46,7 +46,7 @@
 if __name__ == '__main__':
     # Instantiate a test suite containing all the test cases
     suite = unittest.TestSuite()
-    #suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(ReadMetadataTestCase))
+    suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(ReadMetadataTestCase))
     #suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(Bug146313_TestCase))
     #suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(Bug173387_TestCase))
     #suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(Bug175070_TestCase))

=== added file 'test/data/exiv2-bug540.jpg'
Binary files test/data/exiv2-bug540.jpg	1970-01-01 00:00:00 +0000 and test/data/exiv2-bug540.jpg	2010-02-17 01:36:19 +0000 differ