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.

Sign up for GitHub

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

Figure.subplot: Fix strange positioning issues after exiting subplot #2427

Merged
merged 7 commits into from
Mar 16, 2023
Merged

Figure.subplot: Fix strange positioning issues after exiting subplot #2427

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d67810d
Use separate sessions for 'subplot begin' and 'subplot end'
seisman Mar 15, 2023
f262b0c
Improve the comments
seisman Mar 15, 2023
a9eb8fb
Update the grdgradient_shading example to use 'JBC' to position color…
seisman Mar 15, 2023
7e7bf6b
Add a unit test
seisman Mar 15, 2023
0a39369
Minor changes due to different gs versions
seisman Mar 15, 2023
aa40f9c
Update pygmt/tests/test_subplot.py
seisman Mar 16, 2023
dc0fec4
Update baseline image
seisman Mar 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/gallery/images/grdgradient_shading.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,6 @@
panel=True,
)

fig.colorbar(position="x9.0c/-1.1c+w10c/0.25c+h", frame="a2000f500+lElevation (m)")
fig.colorbar(position="JBC+w10c/0.25c+h", frame="a2000f500+lElevation (m)")

fig.show()
11 changes: 8 additions & 3 deletions pygmt/src/subplot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,17 @@ def subplot(self, nrows=1, ncols=1, **kwargs):
"Please provide either one of 'figsize' or 'subsize' only."
)

with Session() as lib:
try:
# Need to use separate sessions for "subplot begin" and "subplot end".
# Otherwise, "subplot end" will use the last session, which may cause
# strange positioning issues for later plotting calls.
# See https://github.com/GenericMappingTools/pygmt/issues/2426.
try:
with Session() as lib:
arg_str = " ".join(["begin", f"{nrows}x{ncols}", build_arg_string(kwargs)])
lib.call_module(module="subplot", args=arg_str)
yield
finally:
finally:
with Session() as lib:
v_arg = build_arg_string({"V": kwargs.get("V")})
lib.call_module(module="subplot", args=f"end {v_arg}")

Expand Down
4 changes: 4 additions & 0 deletions pygmt/tests/baseline/test_subplot_outside_plotting_positioning.png.dvc
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

Does the plot need updating? The unit test is failing at https://github.com/GenericMappingTools/pygmt/actions/runs/4427893754/jobs/7766176078#step:11:704:

__________________ test_subplot_outside_plotting_positioning ___________________
Error: Image files did not match.
  RMS Value: 3.6267715100983793
  Expected:  
    /home/runner/work/pygmt/pygmt/tmp-test-dir-with-unique-name/results/pygmt.tests.test_subplot.test_subplot_outside_plotting_positioning/baseline.png
  Actual:    
    /home/runner/work/pygmt/pygmt/tmp-test-dir-with-unique-name/results/pygmt.tests.test_subplot.test_subplot_outside_plotting_positioning/result.png
  Difference:
    /home/runner/work/pygmt/pygmt/tmp-test-dir-with-unique-name/results/pygmt.tests.test_subplot.test_subplot_outside_plotting_positioning/result-failed-diff.png
  Tolerance: 
    2

Copy link
Member Author

Choose a reason for hiding this comment

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

Minor differences are likely due to different gs versions, but I'm using gs 9.54.0 from conda-forge on macOS and the test passes for me locally:
result-failed-diff

Copy link
Member

Choose a reason for hiding this comment

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

Are you using a dev version of GMT? Can you show your pygmt.show_versions()? This is the one from CI at https://github.com/GenericMappingTools/pygmt/actions/runs/4427893754/jobs/7766177139#step:11:16 which is also using ghostscript 9.54.0:

PyGMT information:
  version: v0.8.1.dev106+g52f04401
System information:
  python: 3.11.0 | packaged by conda-forge | (main, Jan 15 2023, 05:44:48) [Clang 14.0.6 ]
  executable: /Users/runner/miniconda3/envs/pygmt/bin/python
  machine: macOS-12.6.3-x86_64-i386-64bit
Dependency information:
  numpy: 1.24.2
  pandas: 1.5.3
  xarray: 2023.2.0
  netCDF4: 1.6.3
  packaging: 23.0
  contextily: 1.3.0
  geopandas: 0.12.2
  ghostscript: 9.54.0
GMT library information:
  binary version: 6.4.0
  cores: 3
  grid layout: rows
  image layout: 
  library path: /Users/runner/miniconda3/envs/pygmt/lib/libgmt.dylib
  padding: 2
  plugin dir: /Users/runner/miniconda3/envs/pygmt/lib/gmt/plugins
  share dir: /Users/runner/miniconda3/envs/pygmt/share/gmt
  version: 6.4.0

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you using a dev version of GMT?

aha, you're right. I'm using GMT master.

Copy link
Member

Choose a reason for hiding this comment

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

Great, now the GMT Dev Tests are failing on the main branch at https://github.com/GenericMappingTools/pygmt/actions/runs/4443039566/jobs/7799909484#step:17:718 🙃 Should we set an @pytest.mark.xfail?

Copy link
Member Author

Choose a reason for hiding this comment

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

What about increase the RMS diff threshold to 4.0 @pytest.mark.mpl_image_compare(tolerance=4.0)?

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
outs:
- md5: 14da5db0ab63addd8a6d52d6746d9da2
size: 12124
path: test_subplot_outside_plotting_positioning.png
15 changes: 15 additions & 0 deletions pygmt/tests/test_subplot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,3 +100,18 @@ def test_subplot_nrows_ncols_less_than_one_error():
with pytest.raises(GMTInvalidInput):
with fig.subplot(nrows=0, ncols=-1, figsize=("2c", "1c")):
pass


@pytest.mark.mpl_image_compare
def test_subplot_outside_plotting_positioning():
"""
Plotting calls are correctly positioned after exiting subplot.

See https://github.com/GenericMappingTools/pygmt/issues/2426.
seisman marked this conversation as resolved.
Show resolved Hide resolved
"""
fig = Figure()
with fig.subplot(nrows=1, ncols=2, figsize=(10, 5)):
fig.basemap(region=[0, 10, 0, 10], projection="X?", frame=True, panel=True)
fig.basemap(region=[0, 10, 0, 10], projection="X?", frame=True, panel=True)
fig.colorbar(position="JBC+w5c+h", cmap="turbo", frame=True)
return fig