Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork8.1k
[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.
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 commentedJun 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 commentedJun 29, 2013
👍 LGTM - I think this is ready for a merge.@NelleV? |
NelleV commentedJun 29, 2013
👍 |
[sprint] Added mplot3d tests in mpl_toolkits.tests
dmcdougall commentedJun 30, 2013
@NelleV Congratulations on your first merge! |
This complements PR#2135. It should also test issue#1933, which was fixed but untested.