8000 Improve documentation on Axes position by timhoffm · Pull Request #9951 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]

Skip to content

Improve documentation on Axes position #9951

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 1 commit into from
Dec 12, 2017

Conversation

timhoffm
Copy link
Member
@timhoffm timhoffm commented Dec 7, 2017

PR Summary

This PR improves the documentation of:

  • _AxesBase.get_position()
  • _AxesBase.set_position()
  • _AxesBase.reset_position()

Also, it does some minor cleanups on a few other docstrings.

Style questions

  • I would like to reference a documentation section on the figure coordinates. The existing code just references the Figure class, which is not too helpfull.
    Is there are basic description of the coordinate systems other/simpler than the Transforms tutorial?
  • Is there a code style for writing builtins like True and False in the docstring text? Such as in If *True*, return the original position. I've not found anything on this in the numpydocs or in https://github.com/matplotlib/matplotlib/blob/master/doc/devel/documenting_mpl.rst

@anntzer
Copy link
Contributor
anntzer commented Dec 7, 2017

I think we've sort of converged on using double backquotes for True, False and None, which is probably as good as anything (technically single backquotes would also work, as they'd link to, well, the docs entry on docs.python.org for these objects; it's just a matter of consistency).

Copy link
Member
@jklymak jklymak left a comment

Choose a reason for hiding this comment

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

This looks good; suggested change below...

@timhoffm
Copy link
Member Author
timhoffm commented Dec 9, 2017

I've now set True in backqoutes.

@jklymak Thank you for the review. You write "suggested change below". Sorry, I don't know where "below" is. I haven't found the suggested change. Could you please hint, where I have to look?

@WeatherGod
Copy link
Member
WeatherGod commented Dec 9, 2017 via email

@jklymak
Copy link
Member
jklymak commented Dec 9, 2017

oh hmm guess I hit the wrong button somewhere. I’ll look again tonight.

Copy link
Member
@jklymak jklymak left a comment

Choose a reason for hiding this comment

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

Second time's the charm?


The expected shape of ``pos`` is::
There are two position variables:
Copy link
Member

Choose a reason for hiding this comment

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

This was the part I found a bit confusing, and I understand what you are talking about. Maybe:

Axes have two position attributes: the 'original' position is the position allocated for the axes; the 'active' position is the position the Axes is actually drawn at. These positions will usually be the same except if the Axes has its aspect ratio set by .apply_aspect.

Reset the original position to the active position. This resets the
a possible position change due to aspect constraints.
For an explanation of the positions see `.set_position`.
"""
Copy link
Member

Choose a reason for hiding this comment

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

This is actually backwards (as was the original docstring!). Should be

Reset the 'active' position to the 'original' position, undoing the effect of ~.set_aspect.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed to: "Reset the active position to the original position."

It's not really undoing the effect of set_aspect, but of apply_aspect, so I don't want to adapt the second half-sentence. But explaining this is a bit too much for the docstring.

As written in #9964:
Overall, I think we would need a dedicated help page outside the docstrings to explain, coordinates, scaling and the related terminology, which is not always consistent and/or self-explanatory.

Copy link
Member

Choose a reason for hiding this comment

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

I think users will think of it as set_aspect. I also think that set_aspect is the sensible place to define all of this, so a link back to it would be helpful to users. apply_aspect could also be mentioned.

@jklymak jklymak merged commit 40eb83f into matplotlib:master Dec 12, 2017
@QuLogic QuLogic added this to the v2.2 milestone Dec 12, 2017
@timhoffm timhoffm deleted the doc-axes-position branch December 12, 2017 20:39
@QuLogic QuLogic modified the milestones: needs sorting, v2.2.0 Feb 12, 2018
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
0