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

Update Axes docs on aspect-related methods #9964

Merged
merged 1 commit into from Dec 12, 2017

Conversation

Projects
None yet
3 participants
@timhoffm
Copy link
Member

commented Dec 9, 2017

PR Summary

This PR contains updated docstrings on

  • Axes.set_aspect()
  • Axes.apply_aspect()
  • Axes.set_adjustable()
  • Axes.set_anchor()
  • Axes.get_anchor()
@jklymak
Copy link
Contributor

left a comment

This looks very helpful. A few suggestions!

'datalim' change xlim or ylim
'box-forced' same as 'box', but axes can be shared
============ =====================================
Set the aspect of the axis scaling, i.e. the ratio of y-unit to x-unit.

This comment has been minimized.

Copy link
@jklymak

jklymak Dec 9, 2017

Contributor

axis->Axes ?

This comment has been minimized.

Copy link
@timhoffm

timhoffm Dec 10, 2017

Author Member

I think it's really the ratio of y-axis-scaling to x-axis-scaling, not the aspect of the Axes bounding box.

value description
======== ================================================
'auto' automatic; fill position rectangle with data
'equal' same scaling from data to plot units for x and y

This comment has been minimized.

Copy link
@jklymak

jklymak Dec 9, 2017

Contributor

scaling from data to Axes co-ordinates the same for the x and y

This comment has been minimized.

Copy link
@timhoffm

timhoffm Dec 10, 2017

Author Member

This is the original description, which I've not modified. I think both versions are not exactly helpful to the point for different reasons.

Overall, I think we would need a dedicated help page to explain, coordinates, scaling and the related terminology, which is not always consistent and/or self-explanatory.

I prefer to leave it like this for the moment.

======== ================================================
'auto' automatic; fill position rectangle with data
'equal' same scaling from data to plot units for x and y
num a circle will be stretched such that the height

This comment has been minimized.

Copy link
@jklymak

jklymak Dec 9, 2017

Contributor

scaling from data to Axes co-orindates of y dimensions is num times the x dimension. a circle will...

This comment has been minimized.

Copy link
@timhoffm

timhoffm Dec 10, 2017

Author Member

See above.

===== =====================
See Also
--------
matplotlib.axes.Axes.set_adjustable

This comment has been minimized.

Copy link
@jklymak

jklymak Dec 9, 2017

Contributor

Do these need to be made as links?

This comment has been minimized.

Copy link
@jklymak

jklymak Dec 10, 2017

Contributor

Thanks! Deleting comments below just to keep things clean...

Adjust the axes so that it fulfills its aspect setting.
Depending on `.get_adjustable` and `.get_anchor` this will either modify
they axes box or the view limits.

This comment has been minimized.

Copy link
@jklymak

jklymak Dec 9, 2017

Contributor

"...the Axes" (I think we capitalize Axes, maybe incorrect, but I think it makes sense since its not the plural of axis here?)

This comment has been minimized.

Copy link
@QuLogic

QuLogic Dec 10, 2017

Member

We capitalize Axes because it's a Python class and that's PEP8 style. We don't capitalize axes if it's not referring to the class/an instance.

This comment has been minimized.

Copy link
@jklymak

jklymak Dec 10, 2017

Contributor

I think we are referring to an instance here...

This comment has been minimized.

Copy link
@timhoffm

timhoffm Dec 10, 2017

Author Member

Changed this and a few other cases in the above functions.

Note that the distinction is by far not always made in the code. Speaking just of _AxesBase, maybe 2/3 the cases use "axes" where it should be "Axes" according to your rules. IMO this distinction is not too important in many cases, unless you want a strong connotation to the class-type, in which case you would use the reference `Axes` anyway.

This comment has been minimized.

Copy link
@jklymak

jklymak Dec 10, 2017

Contributor

Yeah, I only suggest it because of the linguistic strangeness of referring to "axes" as singular, so at least if we use "Axes" we can say its a singular proper noun.

@timhoffm timhoffm force-pushed the timhoffm:doc-axes-aspect branch from 1f7b450 to a94f682 Dec 10, 2017

@jklymak jklymak merged commit 542f7e6 into matplotlib:master Dec 12, 2017

8 checks passed

ci/circleci: docs-python27 Your tests passed on CircleCI!
Details
ci/circleci: docs-python35 Your tests passed on CircleCI!
Details
codecov/patch Coverage not affected when comparing 14d68df...a94f682
Details
codecov/project/library 61.7% (target 50%)
Details
codecov/project/tests 98.86% remains the same compared to 14d68df
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
lgtm analysis: Python No alert changes
Details
@jklymak

This comment has been minimized.

Copy link
Contributor

commented Dec 12, 2017

Thanks a lot @timhoffm !

@timhoffm timhoffm deleted the timhoffm:doc-axes-aspect branch Dec 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.