Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Use a specific version of Freetype for testing#5306

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
jenshnielsen merged 17 commits intomatplotlib:masterfrommdboom:local-freetype
Nov 5, 2015

Conversation

mdboom
Copy link
Member

This should theoretically allow us to turn down the tolerance on image comparison tests.

If all is going well, this should pass Travis-CI, which is now using a locally-built freetype.

urlretrieve(
'http://download.savannah.gnu.org/releases/freetype/{0}'.format(tarball),
tarball_path)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Could we do a hash check here to make sure that we get an uncorrupted file?

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Good idea.

@jenshnielsen
Copy link
Member

👍

from matplotlib import ft2font
if (ft2font.__freetype_version__ != LOCAL_FREETYPE_VERSION or
ft2font.__freetype_build_type__ != 'local'):
raise ImportError(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Does that mean I can't run tests without this specific freetype installed?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Is it feasible to allow running the tests, just giving a big warning and marking all image comparisons as known-fail? That would still let people run the other tests, or look at the results of single image-comparison tests manually, while not leading them to think the test results are canonical.

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

@QuLogic: You don't need a specific version of freetype installed. You need to build matplotlib with the special "local_freetype" option, which will download and build against a particular version of freetype for you.

@jkseppan: Maybe a warning is the right thing to do -- but knownfail doesn't always communicate "the test run you just ran should give you very little confidence that anything is working" which is really the situation we would have here. What about a warning + real failures?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Warning + failures makes sense to me.

@mdboom
Copy link
MemberAuthor

The experiment in#5307 shows that there are now no text differences, even on Linux vs. Mac. There are other sorts of differences that happen when you turn the tolerance all the way down to 0, however, that I'm now trying to get to the bottom of. Some of it appears to just be random number effects that we've never noticed before with the higher tolerances.

@QuLogic
Copy link
Member

BTW, which tests were different, out of curiosity? Is it mostly Linux vs Mac or even between Linux distros?

@mdboom
Copy link
MemberAuthor

They mostly seem to be differences from one run to the next (all other things being equal). Some things just need an explicit random seed. The others are caused by a very deep and long standing bug in the test runner -- it's possible to write a test returning multiple figures all using the same output filename. Race conditions abound! I hope to have a test for that soon.

@QuLogic
Copy link
Member

Sorry, I meant which tests prompted this PR specifically?

@mdboom
Copy link
MemberAuthor

Oh -- different versions of Freetype have always been a problem since we've had our test suite. It's the whole reason we do "fuzzy" rather than exact matching. But the goal here is to nail down the version of Freetype such that we can do exact matching (and catch more bugs that way).

@QuLogic
Copy link
Member

OK, I'd just like to know whether FreeType eventually stabilized and we could drop this once all the older distros were EOL, or whether it's a more cross-platform issue.

In testing Cartopy, I've never run into anything caused by Freetype and not changes in matplotlib, so perhaps we are all running new (or close) enough versions that it does not trigger anything.

@mdboom
Copy link
MemberAuthor

Freetype has stabilized somewhat in recent years, but we ran into this as recently as 2014 the last time. The cross-platform and cross-distro issues still exist though -- it can be compiled with different hinting settings etc. and this is an attempt to normalize that.

@mdboom
Copy link
MemberAuthor

@QuLogic: Just as an example that this is still a live problem, see#5305.@zLbZ generated a new set of images on Debian testing, and the version of Ubuntu on Travis (I think it's 12.04, maybe 14.04), with freetype 2.6.0 and 2.4.8 respectively, mismatched the images by as much as 20% (notably, these are text-heavy images).

@mdboom
Copy link
MemberAuthor

#5307 is now passing (except for some PEP8 stuff), so I'm convinced this approach to use a fixed version of freetype is sound. This is ready for final review.

@mdboommdboom mentioned this pull requestOct 27, 2015
@QuLogic
Copy link
Member

Ah, I had forgotten about all the configurable hinting settings in FreeType; those are quite likely to cause a sizeable difference (even if mostly imperceptible).

@@ -33,6 +33,22 @@ Optionally you can install:

- `pep8 <http://pep8.readthedocs.org/en/latest>`_ to test coding standards

Build matplotlib for image comparison tests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

The tense here is different from most of the rest of the file (though it's not all totally the same.)

@jenshnielsen
Copy link
Member

Is there any particular reason to use Freetype '2.5.2' According tohttp://www.freetype.org/ several security fixes has gone in since. It seems better to me to use 2.5.5 or 2.6.1?

@mdboom
Copy link
MemberAuthor

@jenshnielsen: I started with the version that is currently on Travis, and the version many (but not all) of the current images were created with, in an attempt to avoid updating too many of the images. Alternatively, we could just move to the latest and start fresh, but that will add more stuff to the repository.

@jenshnielsen
Copy link
Member

AFAIK the version on Travis is 2.4.8

fromhttps://travis-ci.org/matplotlib/matplotlib/jobs/87692893

 freetype: yes [version 2.4.8]

We could upgrade Travis from the existing 12.04 image to the "new" beta 14.04 images to get 2.5.2

This is of cause slightly more complicated because the versions in ubuntu have backported some of the security fixes from 2.5.x

@mdboom
Copy link
MemberAuthor

@jenshnielsen: Ah -- that was my bad -- I just just for what 14.04 had rather than what Travis was actually using. Maybe it would be best to put this at the latest now just for longevity -- the idea should be to rarely, if ever, update this again, except for zero-visible-changes security updates.

@mdboom
Copy link
MemberAuthor

@QuLogic: I've addressed all of your minor points in the latest commits here.

@mdboom
Copy link
MemberAuthor

So, I think before merging, we should have a discussion about which version of Freetype to use here. I could be persuaded either way, I think.

@mdboom
Copy link
MemberAuthor

Rebased.

@WeatherGod
Copy link
Member

Things look good to me. Just waiting for Travis to go green.

jenshnielsen added a commit that referenced this pull requestNov 5, 2015
Use a specific version of Freetype for testing
@jenshnielsenjenshnielsen merged commit0a7a41d intomatplotlib:masterNov 5, 2015
jenshnielsen added a commit to jenshnielsen/matplotlib that referenced this pull requestNov 5, 2015
Use a specific version of Freetype for testing
@jenshnielsen
Copy link
Member

The plan is to backport this to 2.0.x right? In that case feel free to merge#5413

@mdboom
Copy link
MemberAuthor

Yes. This should be merged to 2.0.x. I'll do that.

jenshnielsen added a commit that referenced this pull requestNov 5, 2015
Use a specific version of Freetype for testing
@mdboom
Copy link
MemberAuthor

Cherry-picked to 2.0.x asd730481

@mdboommdboom deleted the local-freetype branchNovember 10, 2015 02:40
@mdboommdboom mentioned this pull requestDec 2, 2015
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Projects
None yet
Milestone
v2.0.0
Development

Successfully merging this pull request may close these issues.

6 participants
@mdboom@jenshnielsen@QuLogic@tacaswell@WeatherGod@jkseppan

[8]ページ先頭

©2009-2025 Movatter.jp