Skip to content
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

FIX: New versions of FreeImage report software as "I", not "ImageMagick ..." #822

Merged
merged 1 commit into from
Nov 20, 2013

Conversation

JDWarner
Copy link
Contributor

This should close both #820 and #730.

It was a bit interesting. It appears some versions of FreeImage - older ones, apparently - reported ('EXIF_MAIN', 'Software') as "ImageMagick [date] [version number] [other stuff]". The old version available through my Enterprise Linux derivative's package manager did this - though it was broken in other ways for being so out of date. Thus, we just checked for the string to start with"ImageMagick"`.

After removing that version of FreeImage, I went and compiled an entirely fresh install from source. And now I can reproduce the errors reported in #820 and #730. New versions of FreeImage are reporting ('EXIF_MAIN', 'Software') as the single character 'I'.

The fix is simple and backwards compatible, just change the test to .startswith('I'), and should be credited to @rovitotv.

I also made a couple minor PEP8 improvements.

@stefanv
Copy link
Member

stefanv commented Nov 20, 2013

Any idea why they did that?

@JDWarner
Copy link
Contributor Author

Not the slightest, just observed the change in behavior. At any rate, this should fix it.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.55%) when pulling 54d7031 on JDWarner:fix_freeimage_metadata_test into f8b5550 on scikit-image:master.

@stefanv
Copy link
Member

stefanv commented Nov 20, 2013

Coverage has the advantage that we now get pinged when new commits are made to PRs we're subscribed to!

stefanv added a commit that referenced this pull request Nov 20, 2013
FIX: New versions of FreeImage report software as "I", not "ImageMagick ..."
@stefanv stefanv merged commit 9d6eee4 into scikit-image:master Nov 20, 2013
@stefanv
Copy link
Member

stefanv commented Nov 20, 2013

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants