-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
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
Conversation
I think one can find gridspec now in the pyplot namespace. |
This should include a link to http://matplotlib.org/users/gridspec.html?highlight=gridspec |
@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 |
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>`_. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps ../../users/gridspec
4bef599
to
693b853
Compare
@phobson this is cool! good use of SG. since the SG PR is about to be merged, could you modify the |
@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 :) |
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 |
@choldgraf I think that's a great idea. I'll rebase and update hopefully later today |
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) |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
@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 ;-) |
@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 |
Two remarks regarding the already existing gridspec tutorial:
|
@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. |
@phobson btw, I found that it can be useful to show off how new content looks on the built docs by using |
9af0fbd
to
e404402
Compare
@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. |
There was a problem hiding this 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 |
There was a problem hiding this comment.
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', |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
# 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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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
I'll rebase this tonight and shift the sections around based on the rest of
the feedback (which is much appreciated!)
-paul
…On Mon, Dec 18, 2017 at 2:43 PM, Chris Holdgraf ***@***.***> wrote:
ping @phobson <https://github.com/phobson> - travis is unhappy FYI. Other
than that, is this ready to go from your end?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#8512 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABHCo1unOEo_O0HNPhlngfo_YZTdgJgGks5tBuqrgaJpZM4NCaBt>
.
|
ping :-) lemme know when the rebase is there |
@phobson popping back onto your queue, as I think this is a great change (but needs a rebase!) |
70a312b
to
14a80bb
Compare
@phobson ping us here when you're ready for another look |
f3b2efd
to
54cd35c
Compare
* add brief quickstart section * remove subplot2grid section * word smithing * added axes to previous examples
54cd35c
to
1352c5c
Compare
@choldgraf @jklymak rebased and made some edits. Off for a bike ride. Back in 6 hours 😉 |
(i.e., ready for another look) |
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? |
...bad indent in |
b7cf955
to
ba1b509
Compare
I fixed(?) the whitespace error in gridspec.py that was propagating to gridspec.rst, but now the build errors are nonsense to me. |
This feels like something that's not related to the PR...any idea what's going on @tacaswell ? Did the travis infrastructure change recently? |
Hard-cycling |
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... |
please work 🤞 |
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 |
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