Skip to content

New gallery example 53 showing subplots and shading - #5336

Merged
maxrjones merged 18 commits into
masterfrom
new-ex53
Jun 25, 2021
Merged

New gallery example 53 showing subplots and shading#5336
maxrjones merged 18 commits into
masterfrom
new-ex53

Conversation

@PaulWessel

Copy link
Copy Markdown
Member

Description of proposed changes

The new example illustrates two useful techniques, one that is new in master:

  1. How to use a simple bash loop to create a subplot with potentially a large set of near-identical panels
  2. How to use the new coast -E+c|C operator for setting up DCW country clip paths

I dumbed the plots down to using the 6m global grids since any courser makes it look weak. The plot using 2m data looks like below:

ex53

Note I also fixed some typos in many of the *.def files hence the many changed files.

@PaulWessel PaulWessel added the documentation Improve documentation label Jun 15, 2021
@PaulWessel PaulWessel added this to the 6.3.0 milestone Jun 15, 2021
@PaulWessel PaulWessel self-assigned this Jun 15, 2021
@maxrjones maxrjones added the add-changelog Add PR to the changelog label Jun 15, 2021

@maxrjones maxrjones left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice!

Comment thread doc/rst/source/gallery/ex53.rst Outdated
Comment thread doc/examples/ex53/ex53.sh
echo ${lon} ${lat} | gmt plot -Skcity/0.25c -Gred -W0.5p -B0
done < $data
gmt subplot end
gmt end show

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The PS file is so large (> 10 Mb).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I know. And that is after I dumbed it down to 6 minutes. As long as we store PS originals I am not sure what we can do about that - never show anything of high resolution? Thoughts?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Perhaps it's time to use the DVC workflow in the GMT project. The DVC workflow is being used in the PyGMT project. It stores all static images (PNG for PyGMT) on another server, and works pretty well for PyGMT. @meghanrjones Thoughts?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

BTW, the PNG version of the above was 11 Mb so I made a JPG (3.7M). Both at 300 dpi default psconvert output.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree that DVC would be a good solution to the PS size issue @seisman. It would require buy-in from all the core developers since it is one more tool to learn for them and would increase the learning curve a bit for new contributors.

@PaulWessel

Copy link
Copy Markdown
Member Author

Note that if I try to be space clever and do this in a much more complicated way:

gmt grdcut @earth_relief_06m -R-1500/1500/-1500/1500+uk -JA${lon}/${lat}/6c -Gz.grd
gmt grdgradient z.grd -A-45 -Nt1 -Gint.grd
gmt coast -E${iso} -M | gmt grdmask -Rint.grd -N0.5/0/0 -Gambient.grd
gmt grdmath int.grd ambient.grd SUB = final_int.grd
gmt grdimage z.grd -R-1500/1500/-1500/1500+uk -JA${lon}/${lat}/? -Ifinal_int.grd

Then it takes way (10x) longer (due to the grdmask step) and bud does make the PS about 6 Mb (due to a single rather than two grdimage calls). I think we don't want to make things complicated when the elegant and faster solution is so much better.

Comment thread doc/rst/source/gallery/ex53.rst Outdated
@joa-quim

Copy link
Copy Markdown
Member

You know that in my opinion the GMT repo is already too huge due to PSs ...

Co-authored-by: Meghan Jones <meghanj@alum.mit.edu>
@PaulWessel

Copy link
Copy Markdown
Member Author

@meghanrjones, I got an email from git about the DISABLE_TEST suggestions but when I click and get here I do not see anything?

Comment thread doc/examples/ex53/ex53.sh
Co-authored-by: Meghan Jones <meghanj@alum.mit.edu>
@maxrjones
maxrjones self-requested a review June 24, 2021 19:02
@seisman

seisman commented Jun 24, 2021

Copy link
Copy Markdown
Member

I think we should also remove the ex53.ps file because it's not used in the tests (GMT_DISABLE_TEST) or the documentation (currently we include a static image directly).

@maxrjones

Copy link
Copy Markdown
Member

I think we should also remove the ex53.ps file because it's not used in the tests (GMT_DISABLE_TEST) or the documentation (currently we include a static image directly).

Yes, I think this is the best intermediate solution. But I did not suggest the proper way to include the static image, so some revisions are still needed in addition to removing the ps file.

Also the tests need to be instructed to skip for GMT_DISABLE_TEST. This is the revised version that Paul suggested adding to gmtest.in L19 to skip these tests:

skip_this_test=$(grep GMT_DISABLE_TEST "$script" -c)
if [ $skip_this_test -gt 0 ]; then	# Do not run this script, probably PS is too alrge
		echo "warning: test disabled for script ${script}." >&2
		exit 0
fi

@maxrjones maxrjones left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I tested that the tests pass and docs build correctly locally with the suggested line changes and the additional changes below.
In doc/rst/source/gallery.rst change

   {% for i in range(1, no_of_examples + 1) %}
   {% set i = '%02d' % i %}
   -  .. figure:: /_images/ex{{i}}.*
         :target: ./gallery/ex{{i}}.html

         :ref:`example_{{i}}`
   {% endfor %}

to

   {% for i in range(1, 53) %}
   {% set i = '%02d' % i %}
   -  .. figure:: /_images/ex{{i}}.*
         :target: ./gallery/ex{{i}}.html

         :ref:`example_{{i}}`
   {% endfor %}

   - .. figure:: https://user-images.githubusercontent.com/14077947/123172626-62ca6880-d44b-11eb-8d91-21e448930460.png
         :target: ./gallery/ex53.html

         :ref:`example_53`
  • Add GMT_DISABLE_TEST lines to gmtest.in
  • remove doc/examples/ex53/ex53.ps

Comment thread doc/examples/ex53/ex53.sh Outdated
Comment thread doc/rst/source/gallery/ex53.rst Outdated
Co-authored-by: Meghan Jones <meghanj@alum.mit.edu>
Co-authored-by: Meghan Jones <meghanj@alum.mit.edu>
@PaulWessel

Copy link
Copy Markdown
Member Author

I see a black rectangle over the bottom half of the PNG figure in vercel...

@maxrjones

Copy link
Copy Markdown
Member

I see a black rectangle over the bottom half of the PNG figure in vercel...

It looks OK for me.

@PaulWessel

Copy link
Copy Markdown
Member Author

OK, then. Shall we squash and merge?

@seisman

seisman commented Jun 24, 2021

Copy link
Copy Markdown
Member

Don't forget to remove the huge PS file!

Comment thread doc/rst/source/gallery.rst
@PaulWessel

Copy link
Copy Markdown
Member Author

Squash and merge when you feel ready.

@maxrjones
maxrjones merged commit fb9f5c3 into master Jun 25, 2021
@maxrjones
maxrjones deleted the new-ex53 branch June 25, 2021 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

add-changelog Add PR to the changelog documentation Improve documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants