assignment of fractions.Fraction to Exif.GPSInfo.GPSAltitude leads to backtrace

Bug #683232 reported by Matěj Cepl
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
pyexiv2
Fix Released
Medium
Olivier Tilloy

Bug Description

With this simple reproducer:

#!/usr/bin/env python

import pyexiv2, fractions

exif = pyexiv2.ImageMetadata('somefile.jpg')
exif.read()

exif['Exif.GPSInfo.GPSAltitude'] = fractions.Fraction(1.5)

on any JPG file, I get this backtrace:

jakoubek:tmp $ python test-pyexiv2-Fraction-bug.py
Traceback (most recent call last):
  File "test-pyexiv2-Fraction-bug.py", line 8, in <module>
    exif['Exif.GPSInfo.GPSAltitude'] = fractions.Fraction(1.5)
  File "/usr/lib64/python2.7/site-packages/pyexiv2/metadata.py", line 261, in __setitem__
    return getattr(self, '_set_%s_tag' % family)(key, tag_or_value)
  File "/usr/lib64/python2.7/site-packages/pyexiv2/metadata.py", line 209, in _set_exif_tag
    tag = ExifTag(key, tag_or_value)
  File "/usr/lib64/python2.7/site-packages/pyexiv2/exif.py", line 104, in __init__
    self._set_value(value)
  File "/usr/lib64/python2.7/site-packages/pyexiv2/exif.py", line 194, in _set_value
    self.raw_value = self._convert_to_string(value)
  File "/usr/lib64/python2.7/site-packages/pyexiv2/exif.py", line 382, in _convert_to_string
    raise ExifValueError(value, self.type)
pyexiv2.exif.ExifValueError: Invalid value for EXIF type [Rational]: [3/2]
jakoubek:tmp $

When the script is modified to use

fraction = fractions.Fraction(1.5)
exif['Exif.GPSInfo.GPSAltitude'] = pyexiv2.Rational(fraction.numerator, fraction.denominator)

instead, there is no backtrace.

Using exiv2-0.20-1.fc14.x86_64 and pyexiv2-0.2.2-2.fc15.x86_64 on Fedora Linux.

Revision history for this message
Matěj Cepl (mcepl) wrote :
Revision history for this message
Robert Bruce Park (robru) wrote :

Thanks for reporting this Matej, I was going to but I'm just a busy guy ;-)

I'm not sure whether I would consider these bugs duplicates or not. Implementing that other one would certainly solve this bug, so they are related, but not quite the same.

The point of this bug is that pyexiv2 has actually suffered a regression somewhere in 0.2 branch, because I used to assign fractions.Fraction all the time in 0.1 without issue, but now it no longer does.

I do very much like the idea of completely ditching pyexiv2.Rational in favor of using only fractions.Fraction, but I understand the implications of the python 2.6 dependency and the reasons for avoiding that, so I would happily settle for just being able to assign a fractions.Fraction _at_all_ even if pyexiv2 internally continues to use pyexiv2.Rational. pyexiv2 should get smarter about converting between the two on the fly as is required internally.

Thanks!

Revision history for this message
Robert Bruce Park (robru) wrote :

The two great benefits of fractions.Fraction over pyexiv2.Rational are:

1. fractions.Fraction can instantiate from a float as of 2.7 (not just a string representing a float), and

2. it provides the method limit_denominator() that introduces a minor amount of rounding in order to make nicer looking fractions.

Real code in my application looks more like:

def float_to_rational(self, value)
    frac = Fraction(abs(value)).limit_denominator(99999)
    return pyexiv2.Rational(frac.numerator, frac.denominator)

And I would greatly prefer if I could do away with this whole method and just assign Fraction objects directly.

Thanks.

Revision history for this message
Olivier Tilloy (osomon) wrote :

I can reproduce reliably (although instantiating a Fraction from a float requires python 2.7, in python 2.6 one needs to pass this float as a string). This is not a duplicate of bug #514415 (even though fixing the latter would fix this issue as a side effect). However this one can probably be fixed fairly easily without requiring the fractions module in python.

Note that I didn’t know pyexiv2 0.1 allowed to assign Fraction objects, that’s an interesting side effect I hadn’t foreseen!

Changed in pyexiv2:
importance: Undecided → Medium
status: New → Confirmed
Olivier Tilloy (osomon)
Changed in pyexiv2:
assignee: nobody → Olivier Tilloy (osomon)
milestone: none → 0.3
status: Confirmed → In Progress
Revision history for this message
Olivier Tilloy (osomon) wrote :

Committed with revision 335 of the trunk.

tags: added: exif xmp
tags: added: regression
Changed in pyexiv2:
status: In Progress → Fix Committed
Revision history for this message
Robert Bruce Park (robru) wrote :

Good stuff Olivier, thanks muchly.

When do you think 0.3 will ship? I'm not impatient, just eager. ;-)

I think it's cute that you didn't even know that 0.1 could handle Fraction objects. My guess is that your 0.1 code was simply accessing numerator and denominator attributes of the Rational object without actually bothering to verify that the object was in fact an instance of pyexiv2.Rational, and since Fraction object supplies the same attributes, it worked just fine, but you never tested it.

Thanks again.

Revision history for this message
Olivier Tilloy (osomon) wrote :

Hey Robert, I think your guess is absolutely correct. I much prefer having strict type-checking and unit tests for all cases, that makes the code more robust. And easy to fix when such a bug is uncovered!

Regarding the release of 0.3, it’s shaping up nicely, I don’t have a set date but I’d really like to have it ship by the end of December. Let’s see how it goes, please keep up with the feedback!

Revision history for this message
Matěj Cepl (mcepl) wrote :

As a consideration for your pain suffering, Robert, here is a scratch build of the tip of pyexiv2 branch as a Fedora 14 package http://koji.fedoraproject.org/koji/taskinfo?taskID=2635693 :) Enjoy.

Revision history for this message
Robert Bruce Park (robru) wrote :

I don't understand this 'scratch build' terminology. Does this mean you backported the fix to 0.2.2 and packaged it for Fedora 14? If so, when will I see this come through in yum?

Sorry for being such a n00b,

Thanks.

Olivier Tilloy (osomon)
Changed in pyexiv2:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.