Wrap subplot using with statement #822

weiji14 · 2021-01-30T07:19:32Z

Description of proposed changes

Yet another attempt at wrapping the subplot function! Preview the tutorial at https://pygmt-git-subplotv3.gmt.vercel.app/tutorials/subplots.html, translated and adapted from https://docs.gmt-china.org/latest/tutorial/subplot/ (in Chinese).

Original GMT subplot function can be found at https://docs.generic-mapping-tools.org/6.1/subplot.html. This implementation is inspired by the inset syntax at #788, namely wrapping things in a with statement like so:

Example code:

import pygmt

fig = pygmt.Figure()
with fig.subplot(nrows=1, ncols=2, figsize=("6c", "3c"), frame="WSne"):
    with fig.set_panel(panel=[0, 0]):
        fig.basemap(region=[0, 3, 0, 3], frame="+tplot0")
    with fig.set_panel(panel=[0,1]):
        fig.basemap(region=[0, 3, 0, 3], frame="+tplot1")
fig.show()

Produces:

Alternatively, this works too:

fig = pygmt.Figure()
with fig.subplot(nrows=1, ncols=2, figsize=("6c", "3c"), frame="WSne"):
    fig.basemap(region=[0, 3, 0, 3], frame="+tplot0", panel=[0, 0])
    fig.basemap(region=[0, 3, 0, 3], frame="+tplot1", panel=[0, 1])
fig.show()

Note that the with statement does gmt subplot begin, and automatically exits using gmt subplot end outside the with-block. Users can set the current subplot using fig.set_panel (similar to matplotlib's plt.sca()) because set is a reserved Python name.

Also cross-referencing Julia wrapper/implementation at https://www.generic-mapping-tools.org/GMT.jl/latest/#GMT.subplot.

fig.subplot() parameters/aliases to wrap:

fig.set_panel() parameters/aliases to wrap:

A = fixedlabel
C = clearance
V = verbose

Fixes #20, Supersedes and closes #412 and closes #427

Reminders

Run make format and make check to make sure the code follows the style guide.
Add tests for new features or tests that would have caught the bug that you're fixing.
Add new public functions/methods/classes to doc/api/index.rst.
Write detailed docstrings for all functions/methods.
If adding new functionality, add an example to docstrings or tutorials.

Notes

You can write /format in the first line of a comment to lint the code automatically

Wrapping the `subplot` function, in a `with` statement! Original GMT `subplot` function can be found at https://docs.generic-mapping-tools.org/6.1/subplot.html. This is my 3rd attempt at implementing `subplot` in PyGMT, with commits heavily re-adapted/cherry-picked from #412 and #427.

Also updated tutorial a bit

willschlitzer · 2021-01-30T11:06:35Z

@weiji14 Any thought to using sca in a context manager? I know it doesn't have a sca_end equivalent the way that inset and subplot do, but I think using it with with statements will make it more clear that the code is creating different subplots, as opposed to a long line of similar looking plotting functions.

seisman · 2021-01-31T07:42:56Z

@weiji14 It's really a nice job! This is almost what I expect, although I haven't read the details.

A quick question about ax and sca. The long-form name of -c option is panel in GMT (see https://github.com/GenericMappingTools/gmt/blob/master/src/gmt_common_longoptions.h). Do we want to follow GMT or matplotlib?

weiji14 · 2021-01-31T08:23:34Z

@weiji14 Any thought to using sca in a context manager? I know it doesn't have a sca_end equivalent the way that inset and subplot do, but I think using it with with statements will make it more clear that the code is creating different subplots, as opposed to a long line of similar looking plotting functions.

I did think of this, but it's technically quite hard to implement. What should a with fig.sca() block reset to? The first subplot? The next subplot? There might be a way using function decorators (?) to insert ax=1 for all fig.* functions under the with block but I'll need more time to confirm that it's possible. Or feel free to suggest ideas :)

@weiji14 It's really a nice job! This is almost what I expect, although I haven't read the details.

Thanks, it's really looking at @willschlitzer's inset wrapper that gave me the inspiration!

A quick question about ax and sca. The long-form name of -c option is panel in GMT (see https://github.com/GenericMappingTools/gmt/blob/master/src/gmt_common_longoptions.h). Do we want to follow GMT or matplotlib?

Tough question. I think panel does seem like a name that conveys what is being done. But ax is shorter and more familiar to Python users :/ If only we could do both! Check with @GenericMappingTools/python and @willschlitzer.

weiji14 · 2021-02-03T04:49:21Z

pygmt/src/subplot.py

+    with Session() as lib:
+        arg_str = " ".join(["set", f"{ax}", build_arg_string(kwargs)])
+        lib.call_module(module="subplot", args=arg_str)
+        yield


@weiji14 Any thought to using sca in a context manager? I know it doesn't have a sca_end equivalent the way that inset and subplot do, but I think using it with with statements will make it more clear that the code is creating different subplots, as opposed to a long line of similar looking plotting functions.

I did think of this, but it's technically quite hard to implement. What should a with fig.sca() block reset to? The first subplot? The next subplot? There might be a way using function decorators (?) to insert ax=1 for all fig.* functions under the with block but I'll need more time to confirm that it's possible. Or feel free to suggest ideas :)

fig.sca is now a context manager as implemented here. There's no try... finally block because there's nothing to reset to. Have a look at the unit tests and tutorials and let me know what you think (i.e. if with fig.sca(): is a good syntax to have, or if we should just stick to fig.sca()).

A problem with this implementation is that any fig.* calls outside the with block would still get applied to the previous panel, which might be counter-intuitive:

with fig.sca(ax=1): fig.basemap(...) # This is put inside panel 1 fig.logo(...) # This also gets put inside panel 1 with fig.sca(ax=2): fig.basemap(...) # This is put inside panel 2

Yes, the fig.sca context manager makes no sense here. I'm wondering if we can ask for upstream changes to be able to reset the ax number?

But that would have to wait until GMT 6.2.0 🙂 Also as mentioned at #822 (comment), what do we reset to? The first subplot? The previous one (-1)?

The first subplot? The previous one (-1)?

Both don't make sense to me. It's weird and confusing to have sca as a context manager, but can add plotting elements to panels outside the context manager.

Yeah, but map elements outside a with fig.sca() block can't just go into the main figure, as it would need to be outside the fig.subplot() indent. It's hard to come up with a good solution here.

So should we 1) just go back to having fig.sca as an ordinary function (i.e. not a context manager)? Or 2) keep fig.sca() as a context manager so that people can organize their subplots using indentation (if they choose to, or not)?

…plot

seisman · 2021-02-06T04:26:53Z

Quick comments. In the 2nd example of the tutorial, you use:

fig.basemap(region=[0, 10, 0, 10], projection="X?", frame=["af", "WSne"], ax=axs[0, 0])

It seems passing the index, or row,col also works,

fig.basemap(region=[0, 10, 0, 10], projection="X?", frame=["af", "WSne"], ax=0)
fig.basemap(region=[0, 10, 0, 10], projection="X?", frame=["af", "WSne"], ax="0,0")

but passing [row, col] doesn't:

fig.basemap(region=[0, 10, 0, 10], projection="X?", frame=["af", "WSne"], ax=[0, 0])

I think we can just set ax as the alias of -c, and convert [0, 0] to 0,0, similar to what we're doing to region (converting [0, 10, 0, 10] to 0/10/0/10.

BTW, it's still unclear to me how ax=0 works in basemap(), as I don't see how you pass -c to basemap, or calling subplot set. Could you please explain it a little bit?

weiji14 · 2021-02-06T04:36:44Z

Quick comments. In the 2nd example of the tutorial, you use:

fig.basemap(region=[0, 10, 0, 10], projection="X?", frame=["af", "WSne"], ax=axs[0, 0])

It seems passing the index, or row,col also works,

fig.basemap(region=[0, 10, 0, 10], projection="X?", frame=["af", "WSne"], ax=0)
fig.basemap(region=[0, 10, 0, 10], projection="X?", frame=["af", "WSne"], ax="0,0")

Yes, in pure GMT, you would use something like -c0 or -c0,1. I'm debating whether to even have the axs numpy array thing, it's meant to be like matplotlib's Axes object but we can't do ax.plot so maybe that will confuse users?

but passing [row, col] doesn't:
fig.basemap(region=[0, 10, 0, 10], projection="X?", frame=["af", "WSne"], ax=[0, 0])
I think we can just set ax as the alias of -c, and convert [0, 0] to 0,0, similar to what we're doing to region (converting [0, 10, 0, 10] to 0/10/0/10.

Ok, I'll read this as aliasing c to ax (rather than using panel), and we just need a sequence (slash) converter in place.

BTW, it's still unclear to me how ax=0 works in basemap(), as I don't see how you pass -c to basemap, or calling subplot set. Could you please explain it a little bit?

ax is an alias for c, so ax=0 is equivalent to -c0 in pure GMT. Using fig.basemap(..., ax=0) would tell the program to directly plot in the first (0) subplot, so subplot set/fig.sca() is not needed. See also https://docs.generic-mapping-tools.org/latest/gmt.html#c-full.

pygmt/src/subplot.py

examples/tutorials/subplots.py

pygmt/tests/test_subplot.py

Add validation checks for inputs to

Co-Authored-By: Dongdong Tian <[email protected]>

pygmt/src/subplot.py

examples/tutorials/subplots.py

seisman · 2021-02-13T22:26:35Z

pygmt/tests/test_subplot.py

@@ -0,0 +1,110 @@
+"""


I'm afraid some of the tests may always pass even though we make some backward-incompatible changes. We can explore how to write better and robust tests later.

Yeah, definitely not my best attempt as it's mostly testing the aliases and a few subtle quirks. There are probably more complicated tests we can write (especially around fig.set_panel), but probably best to stress test things in the real world and make changes as we go along.

Co-Authored-By: Dongdong Tian <[email protected]>

seisman

@weiji14 I believe this PR is very close to merging!

pygmt/src/subplot.py

examples/tutorials/subplots.py

seisman · 2021-02-14T14:01:36Z

pygmt/src/subplot.py

+        follow sequentially. Surround the number or letter by parentheses on
+        any side if these should be typeset as part of the tag. Use **+j|J**\
+        *refpoint* to specify where the tag should be placed in the subplot
+        [TL]. Note: **+j** sets the justification of the tag to *refpoint*


When I review the PR #881 from @meghanrjones, I realize that the default values are not documented well. Most readers may find [TL] confusing, especially that [ ] means optional arguments in GMT's syntax. I feel that we should use [Default is TL], rather than just [TL] for better clarification.

If you agree, then you need to update the docstrings again, and better to document it in the contributing guides.

Suggested change

[TL]. Note: **+j** sets the justification of the tag to *refpoint*

[Default is **TL**]. Note: **+j** sets the justification of the tag to *refpoint*

When I review the PR #881 from @meghanrjones, I realize that the default values are not documented well. Most readers may find [TL] confusing, especially that [ ] means optional arguments in GMT's syntax. I feel that we should use [Default is TL], rather than just [TL] for better clarification.

Yes, I hear you that using [Default is TL] is clearer, but:

There are a lot of these [ ], and it's not easy to CTRL+F and replace all instances.

We have mostly been copying and pasting from upstream GMT, and it will be a lot of work checking through them all (c.f. How to automate copying of GMT documentation to PyGMT #895).

If you agree, then you need to update the docstrings again, and better to document it in the contributing guides.

I think it's ok for [ ] to mean both optional and default as long as it's documented, or like you say, use [Default is TL] instead. I would rather we settle on the standard we want before applying it here in this PR.

OK, we can discuss it later.

Co-Authored-By: Dongdong Tian <[email protected]>

weiji14 · 2021-02-14T20:15:06Z

All green and merged! Thanks @seisman for your patience in all of this 😄

liamtoney · 2021-02-15T08:23:36Z

Big cheers on this @weiji14 — this is going to be a huge feature for many users! 🎉

…Tools#853) Used to select a specific subplot panel. See https://docs.generic-mapping-tools.org/6.1/gmt.html#c-full, https://github.com/GenericMappingTools/gmt/blob/6.1.1/doc/rst/source/explain_-c_full.rst_, and https://docs.generic-mapping-tools.org/6.1/cookbook/options.html#selecting-subplot-panels-the-c-option. Needed for the `subplot` wrapper at GenericMappingTools#822. * Add panel (c) to list of necessary arguments in basemap * Rename alias ax to panel to follow upstream GMT * Update description of common alias c to be more Pythonic Co-authored-by: Dongdong Tian <[email protected]>

Wrapping the `subplot` function, in a `with` statement! Original GMT `subplot` function can be found at https://docs.generic-mapping-tools.org/6.1/subplot.html. This is the 3rd attempt at implementing `subplot` in PyGMT, with commits heavily re-adapted/cherry-picked from GenericMappingTools#412 and GenericMappingTools#427. * Add fig.subplot and fig.set_panel to API docs * Alias fixedlabel (A), clearance (C), verbose (V) for set_panel * Turn fig.set_panel into a context manager * Alias projection (J), region (R), verbose (V), x/yshift (X/Y) for subplot * Allow for spaces in title and labels without needing double quotes Mitigates against GenericMappingTools#247. * Allow for list inputs into fig.set_panel(panel=...) * Rename sca to set_panel and ax to panel * Fix bug that prevented boolean to -A from working * Add note that subplot panel is activated until further notice * Validate subplot nrows/ncols and figsize/subsize argument inputs * Revise advanced subplot layout subsection to use two subplots calls Co-authored-by: Dongdong Tian <[email protected]>

weiji14 force-pushed the subplot_v3 branch from 2e657c0 to f2ac98b Compare January 30, 2021 07:21

vercel bot temporarily deployed to Preview January 30, 2021 07:21 Inactive

weiji14 added the feature Brand new feature label Jan 30, 2021

weiji14 added this to the 0.3.0 milestone Jan 30, 2021

Add fig.subplot and fig.sca to API docs

683ff57

Also updated tutorial a bit

vercel bot temporarily deployed to Preview January 30, 2021 07:30 Inactive

weiji14 added 2 commits February 3, 2021 16:30

Merge branch 'master' into subplot_v3

0742da7

Alias fixedlabel (A), clearance (C), verbose (V) for sca

95e3a64

vercel bot temporarily deployed to Preview February 3, 2021 03:57 Inactive

Turn fig.sca into a context manager

8283f65

vercel bot temporarily deployed to Preview February 3, 2021 04:46 Inactive

weiji14 commented Feb 3, 2021

View reviewed changes

Alias projection (J), region (R), verbose (V), x/yshift (X/Y) for sub…

92b7fbd

…plot

vercel bot temporarily deployed to Preview February 5, 2021 03:28 Inactive

weiji14 added 2 commits February 6, 2021 07:21

Merge branch 'master' into subplot_v3

6cb2e59

Fix subplot end -V

9d6f20b

vercel bot temporarily deployed to Preview February 5, 2021 18:26 Inactive

Improve docstring of fig.subplot and fig.sca

3f0c008

vercel bot temporarily deployed to Preview February 5, 2021 19:57 Inactive

Small tweaks to subplot tutorial

967fba3

vercel bot temporarily deployed to Preview February 5, 2021 20:24 Inactive

weiji14 marked this pull request as ready for review February 5, 2021 20:25

weiji14 requested a review from a team February 6, 2021 03:52

seisman reviewed Feb 13, 2021

View reviewed changes

pygmt/src/subplot.py Outdated Show resolved Hide resolved

pygmt/src/subplot.py Show resolved Hide resolved

pygmt/src/subplot.py Outdated Show resolved Hide resolved

pygmt/src/subplot.py Outdated Show resolved Hide resolved

seisman reviewed Feb 13, 2021

View reviewed changes

examples/tutorials/subplots.py Outdated Show resolved Hide resolved

examples/tutorials/subplots.py Outdated Show resolved Hide resolved

examples/tutorials/subplots.py Outdated Show resolved Hide resolved

examples/tutorials/subplots.py Show resolved Hide resolved

seisman reviewed Feb 13, 2021

View reviewed changes

pygmt/tests/test_subplot.py Outdated Show resolved Hide resolved

weiji14 and others added 2 commits February 14, 2021 08:48

Validate subplot nrows/ncols and figsize/subsize argument inputs

c475861

Add validation checks for inputs to

Proof edits to subplot round 2

a48b600

Co-Authored-By: Dongdong Tian <[email protected]>

vercel bot temporarily deployed to Preview February 13, 2021 19:53 Inactive

weiji14 and others added 2 commits February 14, 2021 09:32

Merge branch 'master' into subplot_v3

2f18fe7

Revise advanced subplot layout subsection to use two subplots calls

f4499a8

Co-Authored-By: Dongdong Tian <[email protected]>

vercel bot temporarily deployed to Preview February 13, 2021 20:35 Inactive

seisman reviewed Feb 13, 2021

View reviewed changes

Proofread edits to subplot round 3

e0900e9

Co-Authored-By: Dongdong Tian <[email protected]>

vercel bot temporarily deployed to Preview February 14, 2021 08:11 Inactive

Replace argument with parameter as per #886

ebc807e

vercel bot temporarily deployed to Preview February 14, 2021 08:58 Inactive

seisman reviewed Feb 14, 2021

View reviewed changes

weiji14 and others added 2 commits February 15, 2021 08:33

Merge branch 'master' into subplot_v3

d16de28

Proofread edits to subplot round 4

30ef9c4

Co-Authored-By: Dongdong Tian <[email protected]>

vercel bot temporarily deployed to Preview February 14, 2021 19:35 Inactive

seisman approved these changes Feb 14, 2021

View reviewed changes

weiji14 merged commit a2628e6 into master Feb 14, 2021

weiji14 deleted the subplot_v3 branch February 14, 2021 20:14

weiji14 mentioned this pull request Apr 5, 2021

Convert booleans arguments in build_arg_string, not in kwargs_to_strings #1125

Merged

5 tasks

weiji14 mentioned this pull request Jul 6, 2021

Expand pygmt subplot's documentation for the figsize and subsize parameters #1370

Open

weiji14 mentioned this pull request Sep 7, 2021

Allow passing arguments containing spaces into pygmt functions #1487

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wrap subplot using with statement #822

Wrap subplot using with statement #822

weiji14 commented Jan 30, 2021 •

edited

Loading

willschlitzer commented Jan 30, 2021

seisman commented Jan 31, 2021

weiji14 commented Jan 31, 2021

weiji14 Feb 3, 2021 •

edited

Loading

seisman Feb 6, 2021

weiji14 Feb 6, 2021

seisman Feb 6, 2021 •

edited

Loading

weiji14 Feb 6, 2021

seisman commented Feb 6, 2021

weiji14 commented Feb 6, 2021

seisman Feb 13, 2021

weiji14 Feb 14, 2021

seisman left a comment

seisman Feb 14, 2021 •

edited

Loading

weiji14 Feb 14, 2021

seisman Feb 14, 2021

weiji14 commented Feb 14, 2021

liamtoney commented Feb 15, 2021

	[TL]. Note: +j sets the justification of the tag to refpoint
	[Default is TL]. Note: +j sets the justification of the tag to refpoint

Wrap subplot using with statement #822

Wrap subplot using with statement #822

Conversation

weiji14 commented Jan 30, 2021 • edited Loading

willschlitzer commented Jan 30, 2021

seisman commented Jan 31, 2021

weiji14 commented Jan 31, 2021

weiji14 Feb 3, 2021 • edited Loading

Choose a reason for hiding this comment

seisman Feb 6, 2021

Choose a reason for hiding this comment

weiji14 Feb 6, 2021

Choose a reason for hiding this comment

seisman Feb 6, 2021 • edited Loading

Choose a reason for hiding this comment

weiji14 Feb 6, 2021

Choose a reason for hiding this comment

seisman commented Feb 6, 2021

weiji14 commented Feb 6, 2021

seisman Feb 13, 2021

Choose a reason for hiding this comment

weiji14 Feb 14, 2021

Choose a reason for hiding this comment

seisman left a comment

Choose a reason for hiding this comment

seisman Feb 14, 2021 • edited Loading

Choose a reason for hiding this comment

weiji14 Feb 14, 2021

Choose a reason for hiding this comment

seisman Feb 14, 2021

Choose a reason for hiding this comment

weiji14 commented Feb 14, 2021

liamtoney commented Feb 15, 2021

weiji14 commented Jan 30, 2021 •

edited

Loading

weiji14 Feb 3, 2021 •

edited

Loading

seisman Feb 6, 2021 •

edited

Loading

seisman Feb 14, 2021 •

edited

Loading