8000 DOC: add quickstart section to the gridspec tutorial by phobson · Pull Request #8512 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]

Skip to content

DOC: add quickstart section to the gridspec tutorial #8512

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 3 commits into from
Jan 19, 2018

Conversation

phobson
Copy link
Member
@phobson phobson commented Apr 20, 2017

PR Summary

This PR adds basic example showing off the gridspec module.
Here's a notebook of the plots: https://gist.github.com/phobson/b087eabac6eda78986bc643e7aaea4ae

PR Checklist

  • Code is PEP 8 compliant
  • Documentation is sphinx and numpydoc compliant

@Tillsten
Copy link
Contributor

I think one can find gridspec now in the pyplot namespace.

@tacaswell
Copy link
Member

This should include a link to http://matplotlib.org/users/gridspec.html?highlight=gridspec

@tacaswell tacaswell added this to the 2.1 (next point release) milestone Apr 20, 2017
@phobson
Copy link
Member Author
phobson commented Apr 20, 2017

@tacaswell Thanks for the link. I couldn't find that last night. Do you know off the top of your head what relative path is at build time? (I'm really bad at sphinx). I can check later today when I'm at home on my mac.

@Tillsten Thanks for the note. I didn't know that. The tutorial Tom linked to uses import matplotlib.gridspec as gridspec, so I'll stay consistent with that.

of axes. The ``gridspec`` module provides more control, but is
proportionally more complex to use.

The API documentation for the ``gridspec`` is `here <users/gridspec>`_.
Copy link
Member Author

Choose a reason for hiding this comment

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

perhaps ../../users/gridspec

@choldgraf
Copy link
Contributor
choldgraf commented Apr 26, 2017

@phobson this is cool! good use of SG. since the SG PR is about to be merged, could you modify the gridspec api page so that it lists the actual classes in the module w/ autosummary (see the patches api page for example, I think it's doc/patches_api and doc/gridspec_api). This will cause the API page to auto-link to the places where it's used in the examples! It's not technically related to this PR but would be a tiny change I think.

@choldgraf
Copy link
Contributor

@phobson actually you know what, this won't work until the other PR is actually merged so I'll just change it there myself to avoid confusion :)

@choldgraf
Copy link
Contributor

OK all the SG PRs are merged now. One quick thought: this seems more like a "tutorial" than an example. What about putting this in tutorials in the intermediate section? At some point it might even be worth spinning off this + the tight_layout tutorial into their own folder called "Controlling Figure / Plot Layouts" or something.

@phobson
Copy link
Member Author
phobson commented May 1, 2017

@choldgraf I think that's a great idea. I'll rebase and update hopefully later today

@story645
Copy link
Member
story645 commented May 1, 2017

How is this different though then from the subplot/gridspec tutorial that's already in there? Or can it be incorporated into it?

import matplotlib.gridspec as gridspec


SIZE = (5, 5)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need to pass SIZE explicitly? it seems irrelevant to this discussion.

Advanced subplot placement with the gridspec module
===================================================

The ``subplots`` function is good for building a uniform grid
Copy link
Contributor

Choose a reason for hiding this comment

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

Should use single backquotes here and below and link to the actual function/module. The additional sentence regarding the API doc would then become unneeded.

@choldgraf
Copy link
Contributor

@story645 I think this is a good point - there's some information that is overlapping and others that isn't. I like the narrative style of this one a bit more, I think it's easier to follow.

I guess one question would be: is there a natural place to split up this tutorial + the other one so that we have 2 tutorials, or alternatively is there a way to incorporate this tutorial into the material of the other? I like @phobson 's style of writing so I am partial to having the other tutorial be improved, but I also don't want to volunteer him for more work ;-)

@phobson
Copy link
Member Author
phobson commented May 1, 2017

@choldgraf perhaps it makes sense to trim this down a bit, and make it a "gridspec quickstart" section of the existing tutorial, along with going over that one's existing content

@anntzer
Copy link
Contributor
anntzer commented May 1, 2017

Two remarks regarding the already existing gridspec tutorial:

  • there are some markup issues.
  • calling gs.update(...) is not needed when you can directly pass the parameters to the constructor.

@choldgraf
Copy link
Contributor

@phobson I think that's a great idea - that tutorial could definitely be made better, and I think that if the structure / content is improved and made more clear, it'll be more effective in the long run than adding a new tutorial. Happy to review etc if you give it a shot.

@choldgraf
Copy link
Contributor

@phobson btw, I found that it can be useful to show off how new content looks on the built docs by using ghp-import to push to gh-pages on your matplotlib fork, and then hosting that with github so you can send links etc

@phobson phobson force-pushed the gridspec-example branch 2 times, most recently from 9af0fbd to e404402 Compare May 2, 2017 05:13
@phobson phobson changed the title DOC: add example showing off basic gridspec usage DOC: add quickstart section to the gridspec tutorial May 2, 2017
@phobson
Copy link
Member Author
phobson commented May 2, 2017

@choldgraf @story645 I took a first pass at merging my example with the existing gridspec tutorial. if you get a sec, please let me know what you think about the direction I'm taking this.

My next steps would be to add a little more explanatory text around the existing examples and unify the coding styles a bit.

Copy link
Member
@story645 story645 left a comment

Choose a reason for hiding this comment

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

Like the direction you're going in here, even learned something from this. I think it's a pretty smooth transition from your guide to the original one.

# instance separately, then pass the elements gridspec instance to the
# :func:`~matplotlib.figure.Figure.add_subplot` method to create the axes
# objects.
# The elements of the gridspec are accessed as if they where elements of
Copy link
Member

Choose a reason for hiding this comment

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

type: were, not where


fig3 = plt.figure()
spec3 = gridspec.GridSpec(ncols=3, nrows=3)
anno_opts = dict(xy=(0.5, 0.5), xycoords='axes fraction',
Copy link
Member

Choose a reason for hiding this comment

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

an explanation, could just be conceptual, of these keywords might be helpful

Copy link
Contributor

Choose a reason for hiding this comment

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

and maybe just add a comment to remind people that add_subplot returns an axes object so you're chaining them together with annotate

@phobson phobson force-pushed the gridspec-example branch from e404402 to 22282ab Compare May 2, 2017 06:11
# For such a simple use case, :mod:`~matplotlib.gridspec` is perhaps overly
# verbose.
# You have to create the figure and :class:`~matplotlib.gridspec.GridSpec`
# instance separately, then pass the elements gridspec instance to the
Copy link
Contributor

Choose a reason for hiding this comment

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

it seems like you've a word

"elements of the" maybe?

# These keyword arguments are lists of numbers.
# Note that absolute values are meaningless, only their relative ratios
# matter.
# That means that ``width_ratios=[2, 4, 8]`` is equivalent to
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure, but will this actually render as a new line in the rST? I think we need an explicit space

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't want a new line (and there is not one in my build). When writing in plaintext, markdown, RST, whatever, I like to put each sentence on a its own line (long senterences might be hard-wrapped or not). So when you make changes, you only have to reflow the sentence, not the paragraph, and don't see any noisey, irrelevant changes to other sentences in the diff.

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool - that makes sense to me! Just noting it to make sure that was intended behavior! (btw, feel free to delete comments of mine that you address or aren't actionable...I tend to do this as a way of ticking off "todo" items in a PR)

# gridspec instance.

gs_kw = dict(width_ratios=widths, height_ratios=heights)
fig5, f5_axes = plt.subplots(ncols=3, nrows=3, gridspec_kw=gs_kw)
Copy link
Contributor

Choose a reason for hiding this comment

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

this is magic, I didn't know you could do this :-P

@phobson
Copy link
Member Author
phobson commented Dec 19, 2017 via email

@choldgraf
Copy link
Contributor

ping :-) lemme know when the rebase is there

@jklymak
Copy link
Member
jklymak commented Jan 9, 2018

@phobson popping back onto your queue, as I think this is a great change (but needs a rebase!)

@choldgraf
Copy link
Contributor

@phobson ping us here when you're ready for another look

  * add brief quickstart section
  * remove subplot2grid section
  * word smithing
  * added axes to previous examples
@phobson
Copy link
Member Author
phobson commented Jan 13, 2018

@choldgraf @jklymak rebased and made some edits.

Off for a bike ride. Back in 6 hours 😉

@phobson
Copy link
Member Author
phobson commented Jan 13, 2018

(i.e., ready for another look)

@choldgraf
Copy link
Contributor

As we already took a look at this before I think we should just merge once the tests are passing. Any idea what's up with circle?

@jklymak
Copy link
Member
jklymak commented Jan 13, 2018

...bad indent in gridspec.py, but hard to debug w/o building locally.

@phobson
Copy link
Member Author
phobson commented Jan 14, 2018

I fixed(?) the whitespace error in gridspec.py that was propagating to gridspec.rst, but now the build errors are nonsense to me.

@choldgraf
Copy link
Contributor

This feels like something that's not related to the PR...any idea what's going on @tacaswell ? Did the travis infrastructure change recently?

@jklymak jklymak closed this Jan 19, 2018
@jklymak jklymak reopened this Jan 19, 2018
@jklymak jklymak closed this Jan 19, 2018
@jklymak
Copy link
Member
jklymak commented Jan 19, 2018

Hard-cycling

@jklymak jklymak reopened this Jan 19, 2018
@jklymak
Copy link
Member
jklymak commented Jan 19, 2018

Hmm, I thought opening closing was supposed to restart the tests, but it didn't restart the doc builds... A little leery about merging w/o the tests...

@phobson
Copy link
Member Author
phobson commented Jan 19, 2018

please work 🤞

@jklymak jklymak merged commit abaedab into matplotlib:master Jan 19, 2018
@choldgraf
Copy link
Contributor

@QuLogic
Copy link
Member
QuLogic commented Jan 19, 2018

CircleCI doesn't test merge commits (which is a bit unfortunate), and that means if you close and re-open, it doesn't restart the test (since the head of the PR hasn't changed). You can restart in the UI, but it still won't see any new things in master.

@phobson phobson deleted the gridspec-example branch January 19, 2018 19:18
@QuLogic QuLogic modified the milestones: needs sorting, v2.2.0 Feb 13, 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.

8 participants
0