Skip to content

[sprint] Added mplot3d tests in mpl_toolkits.tests #2177

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 29, 2013
Merged

[sprint] Added mplot3d tests in mpl_toolkits.tests #2177

merged 1 commit into from
Jun 29, 2013

Conversation

tbekolay
Copy link
Contributor

This complements PR #2135. It should also test issue #1933, which was fixed but untested.

pelson
pelson reviewed Jun 29, 2013
View reviewed changes
lib/mpl_toolkits/tests/test_mplot3d.py
import numpy as np


@image_comparison(baseline_images=['bar3d'])
Copy link
Member

Choose a reason for hiding this comment

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

We might like to remove the text from some of these tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, I heard @WeatherGod justify that yesterday. I'll remove the text from everything except for the text3d example?

Copy link
Member

Choose a reason for hiding this comment

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

Actually, because a lot of special work is done to text in mplot3d, it
might be better to have them in here until a set of tests are made to
specifically exercise them.

@pelson
Copy link
Member

pelson commented Jun 29, 2013

The test failures are not related to this change.

Would you mind running pep8 over the new test and ensuring it meets PEP8 standards (we skip some of the overly strict ones, details in #1945 (comment) from @NelleV.

Other than that - great new addition!

@pelson pelson mentioned this pull request Jun 29, 2013
Closed
@tbekolay
Added mplot3d tests in mpl_toolkits.tests
83da059 
- This complements PR #2135
- Also tests issue #1933, which was fixed but untested
- Also changes the remove_text decorator,
  to remove text on the z-axis.
@pelson
Copy link
Member

pelson commented Jun 29, 2013

👍 LGTM - I think this is ready for a merge. @NelleV?

@NelleV
Copy link
Member

NelleV commented Jun 29, 2013

👍

NelleV added a commit that referenced this pull request Jun 29, 2013
@NelleV
Merge pull request #2177 from tbekolay/mplot3dtests
9efe7c3 
[sprint] Added mplot3d tests in mpl_toolkits.tests
@NelleV NelleV merged commit 9efe7c3 into matplotlib:master Jun 29, 2013
@dmcdougall
Copy link
Member

@NelleV Congratulations on your first merge!

@tacaswell tacaswell mentioned this pull request Aug 17, 2014
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@tbekolay @pelson @NelleV @dmcdougall @WeatherGod