Document mpl_toolkits.axes_grid1.anchored_artists #4874

Merged
merged 9 commits into from Dec 26, 2015

Conversation

Projects
None yet
5 participants
Contributor

sargas commented Aug 6, 2015

Here's another PR following up on #4864.

The documentation isn't built with this commit, but I'll
push another commit (or open another PR if this is already merged) to add the API docs
to doc/mpl_toolkits/axes_grid/api/index.rst once PR #4864 is merged.

tacaswell added this to the next major release (2.0) milestone Oct 8, 2015

@tacaswell tacaswell commented on an outdated diff Nov 5, 2015

lib/mpl_toolkits/axes_grid1/anchored_artists.py
@@ -1,43 +1,152 @@
from __future__ import (absolute_import, division, print_function,
unicode_literals)
-from matplotlib.externals import six
@tacaswell

tacaswell Nov 5, 2015

Owner

please leave the six import even if it isn't used to keep our files consistent.

Owner

mdboom commented Dec 14, 2015

I think this is ready to merge once you address @tacaswell's minor comment.

Contributor

sargas commented Dec 16, 2015

@mdboom sure, but could we hold off until #4864 is merged? It adds a axes_grid1 section to doc/mpl_toolkits/axes_grid/api/index.rst that would be useful to also change in this PR.

tacaswell closed this Dec 16, 2015

tacaswell removed the needs_review label Dec 16, 2015

tacaswell reopened this Dec 16, 2015

Owner

tacaswell commented Dec 16, 2015

merged #4864 cycled open/close to re-trigger travis on new master

Contributor

sargas commented Dec 17, 2015

I've rebased, readded the six import, and enabled the docs to be built in the latest commits.

I did have to change one of the examples which imported matplolib.offsetbox.AnchoredText from mpl_toolkits.axes_grid1.anchored_artists, where it was removed as an unused import. I don't know how API-breaking this is if anyone copied that example.

@tacaswell tacaswell commented on the diff Dec 17, 2015

lib/mpl_toolkits/axes_grid1/anchored_artists.py
@@ -157,73 +364,3 @@ def __init__(self, transform, size, label, loc,
child=self._box,
prop=fontproperties,
frameon=frameon, **kwargs)
-
-
-if __name__ == "__main__":
-
- import matplotlib.pyplot as plt
@tacaswell

tacaswell Dec 17, 2015

Owner

Is it worth moving this code to an example?

@tacaswell tacaswell commented on the diff Dec 17, 2015

lib/mpl_toolkits/axes_grid1/anchored_artists.py
from matplotlib.patches import Rectangle, Ellipse
-import numpy as np
-
-from matplotlib.offsetbox import AnchoredOffsetbox, AuxTransformBox, VPacker,\
@tacaswell

tacaswell Dec 17, 2015

Owner

I am 50/50 on restoring these unused imports.

On one hand it is bad to confuse the API like this by moving classes around between modules.

On the other hand, this is an api break.

In either case 👍 on changing the example to not use this.

@sargas

sargas Dec 17, 2015

Contributor

The code in if __name__ == "__main__": is the only place where AnchoredText and AnnotationBbox are used, so I figured that as the reason for their being imported. Looking further into it, both are imported in lib/mpl_toolkits/axes_grid/anchored_artists.py, so maybe they are meant to be part of the axes_grid{,1} APIs.

Strangely, AnnotationBbox starts in matplotlib.offsetbox in 80fb71a despite being added in axes_grid (AnchoredText is also moved in that commit without explaination). The demo for it doesn't use axes_grid at all.

I'll re-import those classes and add the imports to __all__, since I think a cleanup of the API is outside the scope of this PR.

@jenshnielsen jenshnielsen added a commit that referenced this pull request Dec 26, 2015

@jenshnielsen jenshnielsen Merge pull request #4874 from sargas/cleanup-anchored-artists
Document mpl_toolkits.axes_grid1.anchored_artists
b6a79ac

@jenshnielsen jenshnielsen merged commit b6a79ac into matplotlib:master Dec 26, 2015

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 68.284%
Details

tacaswell removed the needs_review label Dec 26, 2015

@QuLogic QuLogic added a commit to QuLogic/matplotlib that referenced this pull request Oct 16, 2016

@jenshnielsen @QuLogic jenshnielsen + QuLogic Merge pull request #4874 from sargas/cleanup-anchored-artists
Document mpl_toolkits.axes_grid1.anchored_artists

Conflicts:
	lib/mpl_toolkits/axes_grid1/anchored_artists.py

Note that the removed imports were restored for this backport so as to
not break the API for 2.0. Whether they really should be moved or not is
a matter for a different PR.
ef06f5f

QuLogic referenced this pull request Oct 16, 2016

Merged

Backports for 2.0 #7284

Member

QuLogic commented Oct 18, 2016

Backported to v2.x via ef06f5f.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment