Skip to content

Add 3D stem plot #18310

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

Merged
merged 8 commits into from
Dec 25, 2020
Merged

Add 3D stem plot #18310

merged 8 commits into from
Dec 25, 2020

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Aug 21, 2020

PR Summary

This is a rebase and squash of #6271. Because the 3D projection changed since that was written, I had to throw away the original test images to prevent bloat, but I took that opportunity to drop some duplicates and use the mpl20 style. I then modernized docs to match current standards, and simplified some bits.

As this is completely new, I chose to avoid the optional things in the 2D stem, i.e., it does not attempt to guess coordinates for the baseline. It also always returns a Line3DCollection for stemlines.

I'm not entirely sure I like the way zdir works. It uses juggle_axes, so it does like other 2D-converted things and rotates what x/y/z mean. This makes sense for an artist that's a 2D plane, but this is 3D. On the other hand, Line3D works the same way. Perhaps the direction of stem lines/baseline should not be tied to zdir, but use orientation like in 2D?

I will write What's new when we decide what the right API is here.

Closes #6271.

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • [n/a] Documented in doc/api/next_api_changes/* if API changed in a backward-incompatible way

@QuLogic QuLogic added this to the v3.4.0 milestone Aug 21, 2020
@QuLogic QuLogic mentioned this pull request Aug 21, 2020
@QuLogic
Copy link
Member Author

QuLogic commented Aug 26, 2020

I'm not entirely sure I like the way zdir works. It uses juggle_axes, so it does like other 2D-converted things and rotates what x/y/z mean. This makes sense for an artist that's a 2D plane, but this is 3D. On the other hand, Line3D works the same way. Perhaps the direction of stem lines/baseline should not be tied to zdir, but use orientation like in 2D?

On further thought, I think zdir may be the wrong approach, at least as used here. Though it was suggested in #6271, I don't think it does what we want. Namely, it decides, which way is 'up' for a 2D-to-3D conversion. But the parameters of stem3d are 3D already. Thus, using zdir rotates the result, and means you need to juggle around x/y/z if have an existing set and just want the stems in a different orientation starting from a different.

If we want to keep zdir, then I think it should apply only to the bottom baseline. Alternatively, we can have orientation={x, y, z}, which moves only the baseline.

@QuLogic
Copy link
Member Author

QuLogic commented Sep 3, 2020

After discussing with @tacaswell, I changed this to use orientation instead of zdir, and tweaked the tests a bit.

@tacaswell
Copy link
Member

I am 👍 in general, my primary concern is only being able to control the style via the plot-style format strings. I think that this is OK to start with, but long term we may want to expose all of the knobs.

@tacaswell
Copy link
Member

However, this matches the 2D stem (https://matplotlib.org/api/_as_gen/matplotlib.pyplot.stem.html) and you can always change the styling after the fact.

srihitha09 and others added 8 commits December 19, 2020 01:06
* Implemented 3D stem plots.
* refactor code to work with juggle_axes()
* Fixed pep8
* Changed file names
* update code, tests, and demos to work with zdir param and stem()
* fix pep8 for test file
* clean up baseline images

Co-authored-by: Jessy Liang <jessy.liang@mail.utoronto.ca>
Co-authored-by: Samriddhi Kaushik <samriddhi.kaushik@mail.utoronto.ca>
There's no backwards compatibility to worry about, so we don't need to
accept arbitrary positional arguments.
We can stop adding multiple lines and put in a single `plot`, which also
ensures that everything gets the same properties.
This is hopefully a less confusing way to define the 3D stem plot in
different directions, as the x/y/z always mean the same point, while
only the stem direction changes.
These are somewhat like the original tests in matplotlib#6271, but consolidated
into a single image, and testing more properties.
Copy link
Member

@efiring efiring left a comment

Choose a reason for hiding this comment

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

Looks ready to go.

@efiring efiring merged commit c49978d into matplotlib:master Dec 25, 2020
@QuLogic QuLogic deleted the stem3d branch December 25, 2020 02:53
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.

6 participants