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

TST: Make proj3d tests into real tests #7310

Merged
merged 2 commits into from
Oct 25, 2016

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Oct 20, 2016

For some reason, these are buried in mplot3d/proj3d.py instead of in a test file. I guess they're sort of implicitly tested with the rest of the 3D plots, but it's also useful to test them by themselves.

I think the tests are all okay except test_world, because I'm not totally sure what world_transformation is trying to do, but I guess it's okay since it's used for drawing.

@tacaswell tacaswell added this to the 2.1 (next point release) milestone Oct 20, 2016


@image_comparison(baseline_images=['proj3d_axes_cube'], extensions=['png'])
def test_proj_axes_cube():
Copy link
Member

Choose a reason for hiding this comment

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

These might be good candidates for removing the text (so we do not test the tick labels)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, good point; tick labels are unimportant for these tests. I also switched it to the default style because I don't see why new test images need to use the classic style.

pylab.xlim(-0.2, 0.2)
pylab.ylim(-0.2, 0.2)

pylab.show()

def rot_x(V, alpha):
Copy link
Member

@Kojoley Kojoley Oct 20, 2016

Choose a reason for hiding this comment

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

This function is not used internally, exists since 2009 with an error and no one noticed that. Maybe it is better to remove it?

o = (txs[0], tys[0])
ax = (txs[1], tys[1])
ay = (txs[2], tys[2])
az = (txs[3], tys[3])
Copy link
Member

Choose a reason for hiding this comment

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

o, ax, ay, az = zip(txs, tys)

ys = (20, 150)
ax.plot(xs, ys)
points = list(zip(xs, ys))
p0, p1 = points
Copy link
Member

Choose a reason for hiding this comment

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

p0, p1 = points = list(zip(xs, ys)) or even p0, p1 = zip(xs, ys) as points is unused

@WeatherGod
Copy link
Member

I think this is a great idea, and I can't believe I didn't think of this myself. My understanding of the development of mplot3d suggests that these weren't really test functions in the truest sense, but rather debugging tools to help with the original development of the toolkit. But, they do provide useful graphics that I wished I had noticed before, and may be helpful if I ever take up trying to port mplot3d over to the Transforms system.

And thanks for noticing the bug in rot_x(). Again, I think that function might have been used in early implementations of mplot3d to support interactivity but then discarded (or maybe the original author never figured out his bug?).

Bundling them up in the middle of the code ensures they're never run.
I'm pretty sure a rotation should not re-scale w.
@QuLogic
Copy link
Member Author

QuLogic commented Oct 21, 2016

Yea, rot_x definitely seems like some debug stuff, but I left it in just in case it breaks something.

@Kojoley
Copy link
Member

Kojoley commented Oct 21, 2016

rot_x was never used

>git log -S rot_x --source --all
commit edd492e866b0d43b497540f7c0fccc7fbba1021f refs/tags/v0.99.0.rc1
Author: John Hunter <jdh2358@gmail.com>
Date:   Tue Apr 14 14:29:31 2009 +0000

    added mpl_toolkits.mplot3d

    svn path=/trunk/matplotlib/; revision=7041

commit 3e1ff627f01019e52c7cd77912af5a5f66068923 refs/tags/v0.98.1
Author: John Hunter <jdh2358@gmail.com>
Date:   Sun Jun 22 13:52:43 2008 +0000

    removed axes3d

    svn path=/trunk/matplotlib/; revision=5629

commit 3a40365f9d21cdf3375803965c86698065620c16 refs/tags/v0.98.0
Author: John Hunter <jdh2358@gmail.com>
Date:   Thu Mar 16 19:02:00 2006 +0000

    added john porters 3d code

    svn path=/trunk/matplotlib/; revision=2151

@QuLogic
Copy link
Member Author

QuLogic commented Oct 21, 2016

Yes, quite likely. I'll leave it up to @WeatherGod.

@WeatherGod
Copy link
Member

I want to leave it in so that it can be used as a debugging tool in future refactors.

@WeatherGod WeatherGod merged commit de8bc33 into matplotlib:master Oct 25, 2016
@QuLogic QuLogic deleted the mplot3d-proj3d-test branch October 25, 2016 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants