Search code, repositories, users, issues, pull requests...

examples/subplots_axes_and_figures/basic_gridspecs.py Outdated

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

@phobson phobson Apr 26, 2017

Choose a reason for hiding this comment

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

perhaps ../../users/gridspec

@phobson phobson force-pushed the gridspec-example branch from 4bef599 to 693b853 Compare

April 26, 2017 07:10

@phobson phobson added the status: needs review label

Apr 26, 2017

Copy link

Contributor

choldgraf commented Apr 26, 2017 •

edited

Loading

@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.

Copy link

Contributor

choldgraf commented Apr 26, 2017

@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 :)

Copy link

Contributor

choldgraf commented May 1, 2017

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.

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?

anntzer

anntzer reviewed

May 1, 2017

examples/subplots_axes_and_figures/basic_gridspecs.py Outdated

import matplotlib.gridspec as gridspec

SIZE = (5, 5)

Copy link

Contributor

@anntzer anntzer May 1, 2017

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.

anntzer

anntzer reviewed

May 1, 2017

examples/subplots_axes_and_figures/basic_gridspecs.py Outdated

Advanced subplot placement with the gridspec module

===================================================

The ``subplots`` function is good for building a uniform grid

Copy link

Contributor

@anntzer anntzer May 1, 2017

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.

Copy link

Contributor

choldgraf commented May 1, 2017

@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 ;-)

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.

Copy link

Contributor

choldgraf commented May 1, 2017

@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.

Copy link

Contributor

choldgraf commented May 1, 2017

@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 removed the status: needs review label

May 1, 2017

@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 (追記ここまで)

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.

story645

story645 reviewed

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.

tutorials/02_intermediate/gridspec.py Outdated

# 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

@story645 story645 May 2, 2017

Choose a reason for hiding this comment

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

type: were, not where

tutorials/02_intermediate/gridspec.py Outdated

fig3 = plt.figure()

spec3 = gridspec.GridSpec(ncols=3, nrows=3)

anno_opts = dict(xy=(0.5, 0.5), xycoords='axes fraction',

Copy link

Member

@story645 story645 May 2, 2017

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

@choldgraf choldgraf May 2, 2017

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

choldgraf

choldgraf reviewed

tutorials/02_intermediate/gridspec.py Outdated

# 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

@choldgraf choldgraf May 2, 2017

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?

choldgraf

choldgraf reviewed

tutorials/02_intermediate/gridspec.py Outdated

# 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

@choldgraf choldgraf May 2, 2017

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

@phobson phobson May 2, 2017

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

@choldgraf choldgraf May 2, 2017

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)

choldgraf

choldgraf reviewed

tutorials/02_intermediate/gridspec.py Outdated

# 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

@choldgraf choldgraf May 2, 2017

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

Copy link

Member Author

phobson commented Dec 19, 2017 via email

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> .

Copy link

Contributor

choldgraf commented Dec 29, 2017

ping :-) lemme know when the rebase is there

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!)

@phobson phobson force-pushed the gridspec-example branch from 70a312b to 14a80bb Compare

January 13, 2018 07:01

Copy link

Contributor

choldgraf commented Jan 13, 2018

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

@phobson phobson force-pushed the gridspec-example branch from f3b2efd to 54cd35c Compare

January 13, 2018 17:08


 DOC: expand gridspec tutorial

1352c5c

 * add brief quickstart section
 * remove subplot2grid section
 * word smithing
 * added axes to previous examples

@phobson phobson force-pushed the gridspec-example branch from 54cd35c to 1352c5c Compare

January 13, 2018 17:09

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 😉

Copy link

Member Author

phobson commented Jan 13, 2018

(i.e., ready for another look)

Copy link

Contributor

choldgraf commented Jan 13, 2018

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?

Copy link

Member

jklymak commented Jan 13, 2018

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


 DOC: fix whitespace in gridspec tutorial

ba1b509

@phobson phobson force-pushed the gridspec-example branch from b7cf955 to ba1b509 Compare

January 14, 2018 07:55

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.

Copy link

Contributor

choldgraf commented Jan 16, 2018

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

@jklymak jklymak reopened this

@jklymak jklymak closed this

Copy link

Member

jklymak commented Jan 19, 2018

Hard-cycling

@jklymak jklymak reopened this

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...


 small line wrap change to re-trigger tests...

e015f37

Copy link

Member Author

phobson commented Jan 19, 2018

please work 🤞

@jklymak jklymak merged commit abaedab into matplotlib:master