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

Reorganise mpl_toolkits documentation #5752

Merged
merged 32 commits into from
Jan 8, 2016

Conversation

jenshnielsen
Copy link
Member

The mpl_toolkits axes_grid and axisartist documentation is very focused on the axes_grid namespace dispite the fact that this is not recommenced for use. The code has been split into axes_grid1 and axisartist. This PR aims to do the same for the documentation and make the documentation more visible and consistent.

  • Move related packages that we don't ship to external resources
  • Reorganise axes_grid documentation to split in axes_grid1 and axisartist
  • Split examples and figures into 2 folders.
  • Move the documentation up one level in the toc to make it more obvious where the toolkit documentation is.

This is WIP because the text still needs some editing for consistency. A tiny bit of functionality is still hidden in axes_grid.parasiteaxis which I plan to more to axesartist.

I am also tempted to move deprecate axes_grid and interested in input to this.

The docs as build by this branch is up at http://jenshnielsen.github.io/matplotlib

@tacaswell
Copy link
Member

I am not a fan of putting the external toolkits under the same top-level heading. Maybe add a top-level "Third-party toolkits" section?

@jenshnielsen
Copy link
Member Author

I not quite sure what you mean. Are you suggesting to move the page from External Resources to a separate top level heading? That makes sense to me.

@tacaswell
Copy link
Member

Yes, something like:

 - external resources
 - MPL API
 - Toolkits
    - axes_gird1
    - mplot3d
    - 
 - Third-party extensions packages
    - trendvis
    - seaborn
    - ..

@jenshnielsen
Copy link
Member Author

👍

@jenshnielsen jenshnielsen changed the title [WIP] Reorganise mpl_toolkits documentation Reorganise mpl_toolkits documentation Dec 29, 2015
@jenshnielsen
Copy link
Member Author

I think this should be ready for review. The branch at http://jenshnielsen.github.io/matplotlib is updated with the result of building 156cb7f

@WeatherGod
Copy link
Member

Uhm, pretty sure this isn't the result of your changes, but the logo/banner
at the top of the homepage is completely messed up for me. The text of
"matplotlib" is too large causing the "m" and the "b" to be cut off on
either end the radar chart logo to be right on top of it.

Using Firefox 43 on Ubuntu.

On Tue, Dec 29, 2015 at 9:12 AM, Jens Hedegaard Nielsen <
notifications@github.com> wrote:

I think this should be ready for review. The branch at
http://jenshnielsen.github.io/matplotlib is updated with the result of
building 156cb7f
156cb7f


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

@jenshnielsen
Copy link
Member Author

@WeatherGod Is that any different from http://matplotlib.org/devdocs/?

@WeatherGod
Copy link
Member

No, it is the same on both pages.

On Tue, Dec 29, 2015 at 9:41 AM, Jens Hedegaard Nielsen <
notifications@github.com> wrote:

@WeatherGod https://github.com/WeatherGod Is that any different from
http://matplotlib.org/devdocs/?


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

@jenshnielsen
Copy link
Member Author

It's probably due to #5653 then?

@WeatherGod
Copy link
Member

as for the mpl_toolkits page itself:
http://jenshnielsen.github.io/matplotlib/mpl_toolkits/index.html, it feels
like there is too much redundant stuff. For example, there is the "mplot3d"
section header, then there is the "mplot3d" bullet point, and then the
"Matplotlib mplot3d toolkit" sub-bullet point. The axes_grid1 and
axisartist stuff isn't too bad, but still feels a little redundant.

On Tue, Dec 29, 2015 at 9:12 AM, Jens Hedegaard Nielsen <
notifications@github.com> wrote:

I think this should be ready for review. The branch at
http://jenshnielsen.github.io/matplotlib is updated with the result of
building 156cb7f
156cb7f


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

@WeatherGod
Copy link
Member

Most likely, I have posted a comment there.

On Tue, Dec 29, 2015 at 9:44 AM, Jens Hedegaard Nielsen <
notifications@github.com> wrote:

It's probably due to #5653
#5653 then?


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

@jenshnielsen
Copy link
Member Author

@WeatherGod I agree that it's a bit redundant but I have not actually changed that around. That is as it has always been.

@jenshnielsen
Copy link
Member Author

But perhaps we should just remove all the TOCs from that page?

extend and build on Matplotlib functionality,
including a choice of two projection and mapping toolkits
<a href="http://matplotlib.org/basemap">basemap</a> and
<a href="http://scitools.org.uk/cartopy/docs/latest">cartopy</a>
Copy link
Member

Choose a reason for hiding this comment

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

Needs .

Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to link to https://github.com/matplotlib here, since we are encouraging most third-party packages to come under our gh organization?

@mdboom
Copy link
Member

mdboom commented Dec 29, 2015

Oops -- sorry, I missed that this is still a WIP and the text hasn't been edited. I'll hold off on my grammatical/typographical nitpicks until that's done.

@jenshnielsen
Copy link
Member Author

Is not WIP any more but by the looks of it most of the typos are in the original text which I have just moved.

@mdboom
Copy link
Member

mdboom commented Dec 29, 2015

I see. Should we try to fix those now anyway? I'm happy to just do them as a PR against this one if that's more efficient...

@jenshnielsen
Copy link
Member Author

We might as well fix obvious typos now. I have tried not to do to many heavy edits but most of the examples could definitely need an edit but the question is how much effort we want to put into this part of the code?

@mdboom
Copy link
Member

mdboom commented Dec 29, 2015

I'll spend a few minutes (but no more) fixing obvious typos and submit a PR against this one...

@mdboom
Copy link
Member

mdboom commented Dec 29, 2015

Actually, now that I'm understanding the size of this better, I think it's probably best to leave the text as-is for this PR and try to clean it up later. (We really need to devote some real effort to editing the docs for grammar/consistency in a lot of places...)

@jenshnielsen
Copy link
Member Author

@WeatherGod I have removed the toc trees

@mdboom I corrected the typos that you already noted and added a note above the matplotlib organization to thirdpartypackages/index

@jenshnielsen
Copy link
Member Author

I have added an api changes noted that axes_grid is deprecated.

The few files which were different in axes_grid have been copied to axisartist
They are mainly Axes from axes_grid but using axisartist axis. This should really be refactored away to make the Axes take an axis type on construction.

@WeatherGod
Copy link
Member

Has the github.io been updated for the latest changes?

On Wed, Dec 30, 2015 at 7:46 AM, Jens Hedegaard Nielsen <
notifications@github.com> wrote:

I have added an api changes noted that axes_grid is deprecated.

The few files which were different in axes_grid have been copied to
axisartist
They are mainly Axes from axes_grid but using axisartist axis. This
should really be refactored away to make the Axes take an axis type on
construction.


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

@jenshnielsen
Copy link
Member Author

I have uploaded a new preview version to http://jenshnielsen.github.io/matplotlibdocs rather than http://jenshnielsen.github.io/matplotlib

@jenshnielsen
Copy link
Member Author

I think this should be ready to go. The documentation is by no means perfect but I think this is a significant structural improvement.

All functionallity from `mpl_toolkits.axes_grid` can be found in either
`mpl_toolkits.axes_grid1` or `mpl_toolkits.axisartist`. Axes classes from
`mpl_toolkits.axes_grid` based on `Axis` from `mpl_toolkits.axisartist` can be
fund in `mpl_toolkits.axisartist`
Copy link
Member

Choose a reason for hiding this comment

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

"fund" --> "found"

@jenshnielsen
Copy link
Member Author

By the way. I'm happy to revet the deprecation of axes_grid and just change the docs in this PR

The matplotlib :ref:`AxesGrid <toolkit_axesgrid-index>` toolkit is a collection of helper classes to
ease displaying multiple images in matplotlib. The AxesGrid toolkit is
distributed with matplotlib source.
The :ref:`mpl_toolkits.axisgrid1 <toolkit_axesgrid1-index>` toolkit is a
Copy link
Member

Choose a reason for hiding this comment

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

"axisgrid1"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch that is embarrassing

@WeatherGod
Copy link
Member

I think that's all the typos I can find. I vote for deprecation of axes_grid. It has been a long time coming, and splitting axisartist out in the documentation makes things a lot more understandable.

@WeatherGod
Copy link
Member

This has a +1 from me. Should this be backported to v2.x or should it stay in master?

@jenshnielsen
Copy link
Member Author

I was not planning to backport it to 2.x but I can be convinced otherwise. Any objections to merging this. It moves around a large number of examples so I would like to merge it before it acquires to many conflicts.

WeatherGod added a commit that referenced this pull request Jan 8, 2016
Reorganise mpl_toolkits documentation
@WeatherGod WeatherGod merged commit 5eec831 into matplotlib:master Jan 8, 2016
@WeatherGod
Copy link
Member

ok, we will leave it in master for now

@jenshnielsen jenshnielsen deleted the axisgriddocs branch January 8, 2016 14:37
@QuLogic QuLogic modified the milestones: next major release (2.0), proposed next point release (2.1) Jan 8, 2016
QuLogic added a commit to QuLogic/matplotlib that referenced this pull request Oct 19, 2016
These entries were moved from mpl_toolkits to thirdpartpackages on
master as part of matplotlib#5752.
QuLogic added a commit to QuLogic/matplotlib that referenced this pull request Oct 19, 2016
These entries were moved from mpl_toolkits to thirdpartypackages on
master as part of matplotlib#5752.
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.

5 participants