Skip to content

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

Merged
NelleV merged 1 commit intomatplotlib:masterfrom
tbekolay:mplot3dtests
Jun 29, 2013
Merged

[sprint] Added mplot3d tests in mpl_toolkits.tests#2177
NelleV merged 1 commit intomatplotlib:masterfrom
tbekolay:mplot3dtests

Conversation

@tbekolay
Copy link
Copy Markdown
Contributor

@tbekolay tbekolay commented Jun 29, 2013

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

pelson
pelson reviewed Jun 29, 2013
View reviewed changes
Comment thread lib/mpl_toolkits/tests/test_mplot3d.py Outdated
Copy link
Copy Markdown
Member

@pelson pelson Jun 29, 2013

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
Copy Markdown
Contributor Author

@tbekolay tbekolay Jun 29, 2013

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
Copy Markdown
Member

@WeatherGod WeatherGod Jun 29, 2013

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
Copy Markdown
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
Copy Markdown
Member

pelson commented Jun 29, 2013

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

@NelleV
Copy link
Copy Markdown
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
Copy Markdown
Member

dmcdougall commented Jun 30, 2013

@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