missing region border in plots #110

wsavran · 2021-04-01T19:05:00Z

fixes Missing region border when specifying a projection; In addition: a (faster) alternative to using projections #109
added kwargs to plot_basemap to apply 'fast' projection and central latitude
added 'fast' as an option to the plot_args of plot_spatial_dataset and plot_catalog
updated docstrings

- added kwargs to plot_basemap to apply 'fast' projection and central latitude - added 'fast' as an option to the plot_args of plot_spatial_dataset and plot_catalog - updated docstrings

codecov-io · 2021-04-01T19:20:26Z

Codecov Report

Merging #110 (ff0e4da) into master (fa789f0) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #110   +/-   ##
=======================================
  Coverage   53.25%   53.25%           
=======================================
  Files          19       19           
  Lines        3091     3091           
  Branches      445      445           
=======================================
  Hits         1646     1646           
  Misses       1352     1352           
  Partials       93       93

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa789f0...ff0e4da. Read the comment docs.

mherrmann3 · 2021-04-02T08:13:17Z

csep/utils/plots.py


     Returns:
         :class:`matplotlib.pyplot.ax` object

     """
    if ax is None:
        fig = pyplot.figure()
-        ax = fig.add_subplot(111, projection=ccrs.PlateCarree(central_longitude=0.0))
+        ax = fig.add_subplot(111, projection=projection)
+        if apprx:


maybe add & projection == ccrs.PlateCarree()? (not needed if you are sure that plot_basemap() will never be called by a user)

the default arg is projection=ccrs.PlateCarree. I believe its good as Bill left it, to accept different projections. Also the default platecarree looks good, for quick plotting without worrying about projections.

i think plot_basemap will probably be called by other users especially if they are trying to plot catalogs on top of forecasts or other data sets. is there a downside to letting users choose projections in the plot_basemap function?

What I meant is: the 'fast'/apprx "projection" works by rescaling the axes. This rescaling is incompatible with any other "true" (non-PlateCaree) projection, which should be caught by adding & projection == ccrs.PlateCarree() to the if statement.

thats a good point. in the other functions i handled that case by forcing projection=ccrs.PlateCarree(). for example, in plot_catalog(). not sure why i didn't do that in basemap. also, if the user chooses the fast option, we should force PlateCarree().

now that im thinking about it, there are some other parameters that might lose continuity between these cartopy enhanced calls, eg, extent

pabloitu

In func plot_basemap, line

pycsep/csep/utils/plots.py

Line 587 in a604a98

if set_global:

an else is missing:

else:
        ax.set_extent(extents=extent, crs=ccrs.PlateCarree())

so plot_basemap can define the extent, and do not waste downloading time of unneeded basemaps

pabloitu · 2021-04-02T20:22:50Z

csep/utils/plots.py


     Returns:
         :class:`matplotlib.pyplot.ax` object

     """
    if ax is None:
        fig = pyplot.figure()
-        ax = fig.add_subplot(111, projection=ccrs.PlateCarree(central_longitude=0.0))
+        ax = fig.add_subplot(111, projection=projection)
+        if apprx:


the default arg is projection=ccrs.PlateCarree. I believe its good as Bill left it, to accept different projections. Also the default platecarree looks good, for quick plotting without worrying about projections.

pabloitu · 2021-04-02T20:52:16Z

I did a failed commit, reversed it, and the test with jma_catalog_reader is not working anymore (don't understand why really). Sorry the mess.

wsavran · 2021-04-03T16:41:27Z

no worries @pabloitu. i think it is something to do with obspy and nothing that you did. it seems to be failing on an imported docstring. see below.

tests/test_JmaCsvCatalog.py:2: in <module>
    import csep
csep/__init__.py:6: in <module>
    from csep.core import forecasts
csep/core/forecasts.py:13: in <module>
    from csep.core.catalogs import AbstractBaseCatalog
csep/core/catalogs.py:18: in <module>
    from csep.utils.comcat import SummaryEvent
csep/utils/comcat.py:19: in <module>
    from obspy.core.event import read_events
/opt/hostedtoolcache/Python/3.7.10/x64/lib/python3.7/site-packages/obspy/__init__.py:60: in <module>
    _add_format_plugin_table(read, "waveform", "read", numspaces=4)
/opt/hostedtoolcache/Python/3.7.10/x64/lib/python3.7/site-packages/obspy/core/util/base.py:562: in _add_format_plugin_table
    if '%s' in func.__doc__:
E   TypeError: argument of type 'NoneType' is not iterable

wsavran · 2021-04-03T18:05:45Z

csep/utils/plots.py

+    if set_global:
+        ax.set_global()
+    else:
+        ax.set_extent(extents=extent, crs=ccrs.PlateCarree())  # Defined extent always in lat/lon


should the crs here be projection and not ccrs.PlateCarree()

According to the docs: "Set the extent (x0, x1, y0, y1) of the map in the given coordinate system. [scitools.org.uk/cartopy/docs/...]. By default (crs=None), it would take over the projection of the axis (i.e. projection), but since the extent will be - in our case - (always?) specified in the same coordinate system as the original data (i.e. PlateCaree()), it is good as is.

See also stackoverflow.com.

mherrmann3 · 2021-04-04T13:03:22Z

csep/utils/plots.py

+        if apprx:
+            # Re-aspect plot (only for plain matplotlib plots, or when using PlateCarree)
+            LATKM = 110.574
+            ax.set_aspect(1 / (LATKM / 111.320 * numpy.cos(numpy.deg2rad(central_latitude))))


I just noticed that it can be simplified:

ax.set_aspect(111.320 * numpy.cos(numpy.deg2rad(central_latitude)) / LATKM)

Btw: here a reference for why/how this works: stackoverflow.com/a/1253545. I use it essentially for every geographic operation because it allows to go from lat-lon to relative distances in km in a very concise way.
So all in all, you may revise this section to:

# Set plot aspect according to local longitude-to-latitude length ratio # (only compatible with plain PlateCarree "projection") LATKM = 110.574 # length of a ° of latitude [km] [https://stackoverflow.com/a/1253545] ax.set_aspect(111.320 * numpy.cos(numpy.deg2rad(central_latitude)) / LATKM)

Damn. The simplification is wrong; it should be:

ax.set_aspect(111.320 / LATKM / numpy.cos(numpy.deg2rad(central_latitude)))

added missing else statement to handle set_global flag in plot_basemap

wsavran · 2021-04-05T20:21:45Z

heres an example for these different options.

import matplotlib.pyplot as plt
import cartopy.crs as ccrs
from csep.utils.plots import plot_basemap

Plot using `plot_basemap`

ax = plot_basemap(
    'ESRI_terrain', [-100, 30, 0, 80], 
    projection=ccrs.Mercator(), 
    grid=True, grid_labels=True
)

Set extent using `cartopy.crs.PlateCarree()`

This is the current behavior of pycsep.

ax = plot_basemap(
    'ESRI_terrain', [-100, 30, 0, 80],
     projection=ccrs.Mercator(),
     grid=True,
     grid_labels=True
)
ax.set_extent([-100, 30, 0, 80], crs=ccrs.PlateCarree())

Set extent using axes projection

ax = plot_basemap(
    'ESRI_terrain', [-100, 30, 0, 80], 
     projection=ccrs.Mercator(), 
     grid=True, 
     grid_labels=True
)
ax.set_extent([-100, 30, 0, 80], crs=None)

Set extent using same projection as basemap

ax = plot_basemap(
    'ESRI_terrain', [-100, 30, 0, 80], 
     projection=ccrs.Mercator(), 
     grid=True, 
     grid_labels=True
)
ax.set_extent([-100, 30, 0, 80], crs=ccrs.Mercator())

wsavran · 2021-04-06T15:07:57Z

im merging this pr

fixes SCECcode#109: missing region border

a604a98

- added kwargs to plot_basemap to apply 'fast' projection and central latitude - added 'fast' as an option to the plot_args of plot_spatial_dataset and plot_catalog - updated docstrings

wsavran requested a review from pabloitu April 1, 2021 19:05

wsavran mentioned this pull request Apr 1, 2021

Missing region border when specifying a projection; In addition: a (faster) alternative to using projections #109

Closed

wsavran changed the title ~~fixes #109: missing region border~~ missing region border in plots Apr 1, 2021

mherrmann3 reviewed Apr 2, 2021

View reviewed changes

pabloitu reviewed Apr 2, 2021

View reviewed changes

pabloitu force-pushed the bill_fix109_plots branch from 2780d2c to a604a98 Compare April 2, 2021 20:47

wsavran commented Apr 3, 2021

View reviewed changes

mherrmann3 reviewed Apr 4, 2021

View reviewed changes

simplified calculation for the aspect scaling

ff0e4da

added missing else statement to handle set_global flag in plot_basemap

wsavran merged commit a860d08 into SCECcode:master Apr 6, 2021

wsavran deleted the bill_fix109_plots branch April 6, 2021 15:08

mherrmann3 mentioned this pull request Jun 8, 2021

Fix simplification of lat-to-lon aspect ratio calculation #125

Merged

mherrmann3 mentioned this pull request Jun 21, 2021

Fix lat-to-lon aspect ratio (now for real) #127

Merged

missing region border in plots #110

missing region border in plots #110

Uh oh!

Conversation

wsavran commented Apr 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Apr 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wsavran Apr 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pabloitu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pabloitu commented Apr 2, 2021

Uh oh!

wsavran commented Apr 3, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wsavran commented Apr 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Plot using plot_basemap

Set extent using cartopy.crs.PlateCarree()

Set extent using axes projection

Set extent using same projection as basemap

Uh oh!

wsavran commented Apr 6, 2021

Uh oh!

Uh oh!

wsavran commented Apr 1, 2021 •

edited

Loading

codecov-io commented Apr 1, 2021 •

edited

Loading

wsavran Apr 3, 2021 •

edited

Loading

wsavran commented Apr 5, 2021 •

edited

Loading

Plot using `plot_basemap`

Set extent using `cartopy.crs.PlateCarree()`