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

Backport #5307 to v2.0.x #5652

Merged
merged 1 commit into from
Dec 13, 2015

Conversation

mdboom
Copy link
Member

@mdboom mdboom commented Dec 10, 2015

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

@mdboom
Copy link
Member Author

mdboom commented Dec 10, 2015

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

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

QuLogic commented Dec 11, 2015

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
Copy link
Member Author

mdboom commented Dec 11, 2015

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
Copy link
Member Author

mdboom commented Dec 11, 2015

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 mdboom force-pushed the backport-lower-tolerance-2.0 branch from 61cc364 to 65dc69a Compare December 11, 2015 15:52
@mdboom
Copy link
Member Author

mdboom commented Dec 11, 2015

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
Copy link
Member Author

mdboom commented Dec 11, 2015

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 mdboom force-pushed the backport-lower-tolerance-2.0 branch from 65dc69a to 57c33f0 Compare December 11, 2015 20:08
@QuLogic
Copy link
Member

QuLogic commented Dec 11, 2015

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
Copy link
Member Author

mdboom commented Dec 11, 2015

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 mdboom force-pushed the backport-lower-tolerance-2.0 branch from 57c33f0 to a977b8f Compare December 11, 2015 21:24
@mdboom
Copy link
Member Author

mdboom commented Dec 11, 2015

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
Copy link
Member Author

mdboom commented Dec 11, 2015

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

@tacaswell
Copy link
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
Copy link
Member Author

mdboom commented Dec 13, 2015

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
Copy link
Member Author

mdboom commented Dec 13, 2015

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

@tacaswell
Copy link
Member

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

@tacaswell
Copy link
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 mdboom force-pushed the backport-lower-tolerance-2.0 branch from 8ccbe8c to 164222a Compare December 13, 2015 18:43
@mdboom
Copy link
Member Author

mdboom commented Dec 13, 2015

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.

@tacaswell
Copy link
Member

Ah, sorry, I was confused

tacaswell added a commit that referenced this pull request Dec 13, 2015
@tacaswell tacaswell merged commit 17a3a6d into matplotlib:v2.0.x Dec 13, 2015
@mdboom
Copy link
Member Author

mdboom commented Dec 13, 2015

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

@QuLogic QuLogic mentioned this pull request Oct 16, 2016
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