ENH: add pivot kwarg to 3d quiver plot#3735
Conversation
Also removing an unused inner function and cleaning up some math.
There was a problem hiding this comment.
Might as well turn this into for xyz, uvw in zip(zip(xs, ys, zs), zip(us, vs, ws)): Which I could have sworn got done when this came in...
|
tagged as revision as it needs tests. Probably just png for the image tests. |
|
I vectorized and simplified quite a bit, possibly at the expense of readability. I tried to comment the tricky bits, though. Tests will come soon. |
|
Tests are in and Travis is happy. Anything else need to be done for this PR? |
|
👍 from me. @efiring Should probably take a look at this. Is there any noticeable speed up? |
There was a problem hiding this comment.
spaces after the commas, please
There was a problem hiding this comment.
Curious travis didn't catch that...
There was a problem hiding this comment.
That is because mplot3d is not pep8 checked. The toolkits generally are not
checked.
On Wed, Oct 29, 2014 at 1:42 PM, Thomas A Caswell [email protected]
wrote:
In lib/mpl_toolkits/mplot3d/axes3d.py:
linec = art3d.Line3DCollection(lines, *args[argi:], **kwargs) self.add_collection(linec)
self.auto_scale_xyz(xs, ys, zs, had_data)self.auto_scale_xyz(XYZ[:,0], XYZ[:,1], XYZ[:,2], had_data)Curious travis didn't catch that...
—
Reply to this email directly or view it on GitHub
https://github.com/matplotlib/matplotlib/pull/3735/files#r19556818.
|
Thank you very much for your feature, and for vectorizing much of the code. In the initial PR, we discussed vectorizing, but the students who developed the feature could only get so far, so thank you for getting it all the way. It is much cleaner to read now. |
|
Ok, linting done. I also did a quick benchmark using this setup code: Before this PR: After this PR: Looks like a 5x speedup! Not bad. |
|
@perimosocordiae One last thing, can you add an entry to |
[ci skip]
|
Ok, added. |
ENH: add pivot kwarg to 3d quiver plot
|
Thank you. |
Also removing an unused inner function and cleaning up some math.
The default value for
pivotis "tip", which preserves the behavior with the existing 3d quiver plot, but differs from the regular 2d version which defaults to "tail".I suspect this PR will need tests added, but I'll need help setting those up.