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

Add documentation for mpl_toolkits.axes_grid1.inset_locator #4864

Merged
merged 13 commits into from Dec 16, 2015

Conversation

sargas
Copy link
Contributor

@sargas sargas commented Aug 4, 2015

This module has some nice, undocumented (except in narrative form) helpers that has been interesting to figure out.

I made some formatting changes, and some slight API differences:

  1. AnchoredSizeLocator and AnchoredZoomLocator both overrode __call__ merely to save the axes (which is unused in AnchoredSizeLocator anyways). I moved the saving of this value to the __call__ of their superclass. No other file in matplotlib uses these classes, and the superclass doesn't look used for anything else online.
  2. inset_axes and zoomed_inset_axes take a **kwarg, but the only keyworded argument that would not result in an error is borderpad. I changed the functions to make that clear.

I still have AnchoredLocatorBase (and its subclasses) to document, but I don't understand the code as is - for instance, a required parameter offsetbox is always unused, and I don't understand why a locator object (something used with Axes.set_axes_locator) would ever have any method except those used by __call__ (making the capabilities inherited from AnchoredOffsetBox useless).

@sargas sargas changed the title Add documentation for mpl_toolkits.axes_grid1 Add documentation for mpl_toolkits.axes_grid1.inset_locator Aug 4, 2015
@WeatherGod
Copy link
Member

Docbuild failure, but I don't know what it means: https://api.travis-ci.org/jobs/74123925/log.txt?deansi=true

@sargas
Copy link
Contributor Author

sargas commented Aug 5, 2015

I generated the docs in py3 on my local machine, it turns out the dedent_interpd decorator doesn't like to work on classes in py2.

I moved the class-level docstrings to the __init__ functions; that allows the docs to build here.
I don't know how the python3.3 error (in matplotlib.tests.test_backend_ps.test_savefig_to_stringio_with_usetex) could have happened though.

@WeatherGod
Copy link
Member

Ok, that makes sense. Don't worry about that usetex test, it is a transient
one that has to do with the Travis system.
On Aug 4, 2015 8:59 PM, "Joseph Booker" notifications@github.com wrote:

I generated the docs in py3 on my local machine, it turns out the
dedent_interpd decorator doesn't like to work on classes in py2.

I moved the class-level docstrings to the init functions; that allows
the docs to build here.
I don't know how the python3.3 error (in
matplotlib.tests.test_backend_ps.test_savefig_to_stringio_with_usetex)
could have happened though.


Reply to this email directly or view it on GitHub
#4864 (comment)
.

@tacaswell
Copy link
Member

Awesome!

I have no problem with this sneaking in under the wire for 1.5, but do not have time to review.

@tacaswell tacaswell added this to the proposed next point release milestone Aug 5, 2015
@WeatherGod
Copy link
Member

A question for the other devs... Is there any reason why we continue to have the axes_grid and axes_grid1? From my understanding, axes_grid is "deprecated" and is only kept around for compatibility. Documentation-wise, it looks like only the axes_grid module is actually generated.

@jenshnielsen
Copy link
Member

I would be 👍 on removing axes_grid

@tacaswell
Copy link
Member

Has anyone been able to review this?

@tacaswell
Copy link
Member

@leejjoon Can you review this?

from __future__ import (absolute_import, division, print_function,
unicode_literals)

from matplotlib.externals import six
Copy link
Member

Choose a reason for hiding this comment

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

please leave the six import

@tacaswell
Copy link
Member

Gave this a quick read, looks really good 👍

There are a bunch of places where things need to be put under a Notes section for numpydoc to render them nicely.

In the places where **kwargs is eliminated, it should be checked that all possible keyword args really are covered.

@sargas
Copy link
Contributor Author

sargas commented Nov 18, 2015

Thanks for reviewing this.

I don't think the kwargs should go into Notes, since the keyworded arguments are parameters.

I attempted to do the following in Parameters:

    kwargs
        Patch properties for the lines and box drawn:
        %(Patch)s

This doesn't quite get the formatting right either (the start of the description is misaligned with the start of the descriptions of the other parameters). The generated HTML has an extra <blockquote> element causing this.

@tacaswell
Copy link
Member

I think the priority should be that the docs render reasonably even if they are not semantically correct.

@sargas
Copy link
Contributor Author

sargas commented Nov 18, 2015

Okay. I've also double checked the **kwargs options I removed.

tacaswell added a commit that referenced this pull request Dec 16, 2015
DOC: Add documentation for mpl_toolkits.axes_grid1.inset_locator
@tacaswell tacaswell merged commit 4573e1e into matplotlib:master Dec 16, 2015
@tacaswell tacaswell modified the milestones: next major release (2.0), proposed next point release (2.1) Dec 16, 2015
@tacaswell
Copy link
Member

This is what the built docs look like http://matplotlib.org/devdocs/mpl_toolkits/axes_grid/api/inset_locator_api.html

tacaswell added a commit that referenced this pull request Dec 16, 2015
DOC: Add documentation for mpl_toolkits.axes_grid1.inset_locator
@tacaswell
Copy link
Member

back-ported to 1.5.x as 8677af6

@sargas sargas deleted the document_inset_locator branch December 16, 2015 17:36
@QuLogic QuLogic modified the milestones: Critical bugfix release (1.5.1), next major release (2.0) Dec 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants