Skip to content

Add a get_zaxis method for 3d axes. #11385

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

Conversation

ImportanceOfBeingErnest
Copy link
Member

PR Summary

Because the normal axes.Axes have a .get_xaxis and .get_yaxis method, which are exposed by the Axes3D, there should be a get_zaxis method as well.

This adds a get_zaxis method to the Axes3D.

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 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)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@ImportanceOfBeingErnest ImportanceOfBeingErnest force-pushed the get_zaxis_for_3d_plot branch 5 times, most recently from c7edaf8 to 6f42f2b Compare June 6, 2018 12:42
Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

Modulo minor docstring issues.


for ax in self.xaxis, self.yaxis, self.zaxis:
ax.init3d()

def get_zaxis(self):
'''Return the :class:`ZAxis<~.axis3d.Axis>` instance.'''
Copy link
Member

Choose a reason for hiding this comment

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

Does not link in the docs.

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. Any idea why?

Copy link
Member

Choose a reason for hiding this comment

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

For web links there must be a space between the display text and <. Maybe it's that.

I don't know if this format is supported by (class)-roles (never seen this use-case before).

Copy link
Member Author

@ImportanceOfBeingErnest ImportanceOfBeingErnest Jun 6, 2018

Choose a reason for hiding this comment

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

I see why. axis3d.Axis does not have any docstring. I guess I will add one.

Must be something else. Confused.

Copy link
Member Author

Choose a reason for hiding this comment

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

That seems to be a fundamental problem. How do you link to a code element like a class using a text that is not the class name itself?

Copy link
Member

@timhoffm timhoffm Jun 6, 2018

Choose a reason for hiding this comment

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

I think :py:meth:`Queue.get <Queue.Queue.get>` worked for me at some point. At least, I've written that in sphinx-doc/sphinx#4416. 😏

Try :class:`ZAxis <mpl_toolkits.mplot3d.axis3d.Axis>` or Z\ `.Axis` . In the latter case, be sure to make the docstring a raw string.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will go with this now:

image

@ImportanceOfBeingErnest ImportanceOfBeingErnest force-pushed the get_zaxis_for_3d_plot branch 3 times, most recently from 021fa8a to e7c3b00 Compare June 6, 2018 19:52
@anntzer anntzer force-pushed the get_zaxis_for_3d_plot branch from e7c3b00 to 664ccc8 Compare January 13, 2019 16:46
@anntzer
Copy link
Contributor

anntzer commented Jan 13, 2019

@ImportanceOfBeingErnest allowed myself to rebase this for you. Will merge after CI passes.

@anntzer anntzer merged commit 5cdd2e5 into matplotlib:master Jan 13, 2019
@ImportanceOfBeingErnest ImportanceOfBeingErnest deleted the get_zaxis_for_3d_plot branch January 13, 2019 17:49
@QuLogic QuLogic added this to the v3.1 milestone Jan 14, 2019
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.

5 participants