FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Backport #5307 to v2.0.x by mdboom · Pull Request #5652 · matplotlib/matplotlib · GitHub

Repository navigation

Backport #5307 to v2.0.x - #5652

Merged
tacaswell merged 1 commit into
matplotlib:v2.0.xfrom
mdboom:backport-lower-tolerance-2.0
Dec 13, 2015
Merged

tacaswell merged 1 commit into
matplotlib:v2.0.xfrom
mdboom:backport-lower-tolerance-2.0

Conversation

mdboom commented Dec 10, 2015

Copy link
Copy Markdown
Member

Running this through a PR to give it proper Travis testing.

mdboom commented Dec 10, 2015

Copy link
Copy Markdown
Member Author

This is green on Travis, and therefore probably good to merge.

mdboom closed this Dec 10, 2015
mdboom reopened this Dec 10, 2015
QuLogic added this to the next major release (2.0) milestone Dec 11, 2015

QuLogic commented Dec 11, 2015

Copy link
Copy Markdown
Member

To trim this down a bit, the text changes seem consistent with those on master. There are a few different images:

git di --shortstat backport-lower-tolerance-2.0..master -- lib/matplotlib/tests/baseline_images/ lib/mpl_toolkits/tests/baseline_images/
 17 files changed, 5853 insertions(+), 5742 deletions(-)

which are mostly non-existent on 2.0, but there are a few more other changes.

lib/matplotlib/tests/baseline_images/test_artist/default_edges.png and lib/matplotlib/tests/baseline_images/test_transforms/pre_transform_data.{png,pdf,svg} are different due to altered view limits around some collections. I think that's from #5583, but I'm wondering why that's not also on master.

I'm not sure why lib/mpl_toolkits/tests/baseline_images/test_axes_grid1/divider_append_axes.svg is different.

I don't understand why lib/matplotlib/tests/baseline_images/test_axes/dash_offset.{pdf,png} exist only on master, but dash_offset.svg exists on this branch too. I don't think it should have been added here.

mdboom commented Dec 11, 2015

Copy link
Copy Markdown
Member Author

Thanks for looking at this.

I think #5583 never got fully backported to 2.0.x. I've done that, now I'll rebase this on the new 2.0.x and see how that pans out.

mdboom commented Dec 11, 2015

Copy link
Copy Markdown
Member Author

I don't understand why lib/matplotlib/tests/baseline_images/test_axes/dash_offset.{pdf,png} exist only on master, but dash_offset.svg exists on this branch too. I don't think it should have been added here.

I'm not sure I follow. It exists in both places, AFAICT.

mdboom force-pushed the backport-lower-tolerance-2.0 branch from 61cc364 to 65dc69a Compare December 11, 2015 15:52

mdboom commented Dec 11, 2015

Copy link
Copy Markdown
Member Author

I've redone this after backporting #5583 to v2.0.x and there were far fewer conflicts this time -- mainly just in tests that exist on master but not 2.0.x.

mdboom commented Dec 11, 2015

Copy link
Copy Markdown
Member Author

I found two other PRs that should have been backported to 2.0.x but weren't (we probably need a better workflow for that, but it is what it is for the present time). These PRs were #5501 and #5433.

mdboom force-pushed the backport-lower-tolerance-2.0 branch from 65dc69a to 57c33f0 Compare December 11, 2015 20:08

QuLogic commented Dec 11, 2015

Copy link
Copy Markdown
Member

It exists in both places, AFAICT.

They didn't exist when I looked ;)

I'm not sure why lib/mpl_toolkits/tests/baseline_images/test_axes_grid1/divider_append_axes.svg is different.

This file is still different. Maybe including #5651 here will make them more consistent?

These PRs were #5501 and #5433.

For some reason, GitHub is not show the backport of #5433 here even though it does say it's part of this PR if you go to your fork and look at the commit directly. Anyway...

mdboom commented Dec 11, 2015

Copy link
Copy Markdown
Member Author

For some reason, GitHub is not show the backport of #5433 here even though it does say it's part of this PR if you go to your fork and look at the commit directly.

#5433 was backported directly onto v2.0.x, and then this PR was based on that.

mdboom force-pushed the backport-lower-tolerance-2.0 branch from 57c33f0 to a977b8f Compare December 11, 2015 21:24

mdboom commented Dec 11, 2015

Copy link
Copy Markdown
Member Author

I'm not sure why lib/mpl_toolkits/tests/baseline_images/test_axes_grid1/divider_append_axes.svg is different.
This file is still different. Maybe including #5651 here will make them more consistent?

Unless I'm misunderstanding, I think this is no longer the case. Different from what? On this branch vs. upstream/master?

git diff master -- lib/mpl_toolkits/tests/baseline_images/test_axes_grid1/divider_append_axes.svg

mdboom commented Dec 11, 2015

Copy link
Copy Markdown
Member Author

I think this is green again. @QuLogic: I think I've addressed all that you found, but let me know if otherwise.

Copy link
Copy Markdown
Member

I am seeing 3 files different

 $git diff --stat matplotlib/master -- lib/mpl_toolkits/tests/baseline_images/
 lib/mpl_toolkits/tests/baseline_images/test_axes_grid1/divider_append_axes.svg         |  3714 ++++++++--------
 lib/mpl_toolkits/tests/baseline_images/test_axes_grid1/twin_axes_empty_and_removed.png |   Bin 37749 -> 0 bytes
 lib/mpl_toolkits/tests/baseline_images/test_mplot3d/mixedsubplot.svg                   | 16034 +++++++++++++++++++++++++++++++++++-----------------------------------
 3 files changed, 9874 insertions(+), 9874 deletions(-)

mdboom commented Dec 13, 2015

Copy link
Copy Markdown
Member Author

I'll just forcibly pick those 3 files from master (since I think the differences are all non-visible anyway) and see how that goes...

mdboom commented Dec 13, 2015

Copy link
Copy Markdown
Member Author

Ok. I think we're finally good-to-go here.

Copy link
Copy Markdown
Member

I don't see where those other 3 files got pulled in

Copy link
Copy Markdown
Member

maybe #5621 should be backported to 2.0.x and then this backport rebased on top of it? That is the source of the missing png

mdboom force-pushed the backport-lower-tolerance-2.0 branch from 8ccbe8c to 164222a Compare December 13, 2015 18:43

mdboom commented Dec 13, 2015

Copy link
Copy Markdown
Member Author

I don't see where those other 3 files got pulled in

I checked them out directly from master and amended the commit it order to reduce the amount of large diff.

maybe #5621 should be backported to 2.0.x and then this backport rebased on top of it? That is the source of the missing png

Sure. Done.

Copy link
Copy Markdown
Member

Ah, sorry, I was confused

tacaswell added a commit that referenced this pull request Dec 13, 2015
tacaswell merged commit 17a3a6d into matplotlib:v2.0.x Dec 13, 2015

mdboom commented Dec 13, 2015

Copy link
Copy Markdown
Member Author

No worries -- I was confused as well. This massive PRs are hard to manage...

QuLogic mentioned this pull request Oct 16, 2016
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

3 participants


Back | FazBrowse Home | New Git URL