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.

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

Add Figure.tilemap to plot XYZ tile maps #2394

Merged
merged 24 commits into from
Mar 27, 2023
Merged

Add Figure.tilemap to plot XYZ tile maps #2394

merged 24 commits into from
Mar 27, 2023

Conversation

weiji14
Copy link
Member

@weiji14 weiji14 commented Mar 4, 2023

Description of proposed changes

New tilemap function for plotting XYZ tile maps! This is essentially a convenience wrapper around pygmt.datasets.load_tile_map and pygmt.Figure.grdimage.

Preview at https://pygmt-dev--2394.org.readthedocs.build/en/2394/api/generated/pygmt.Figure.tilemap.html

This is the second part of #2115 that takes the output xarray.DataArray from pygmt.datasets.load_tile_map (added in #2125) and plots it using fig.grdimage.

Usage:

import contextily
import pygmt

fig = pygmt.Figure()
fig.tilemap(
    region=[-157.6, -157.1, 1.68, 2.08],  # West, East, South, North
    source=contextily.providers.Stamen.Watercolor,
    frame=True,
)
fig.show()

produces this watercolor map of Kiribati:

Kiribati_watercolor_map

Notes:

Fixes #2115

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 wrapping a new module, open a 'Wrap new GMT module' issue and submit reasonably-sized PRs.
  • If adding new functionality, add an example to docstrings or tutorials.
  • Use underscores (not hyphens) in names of Python files and directories.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash commands are:

  • /format: automatically format and lint the code
  • /test-gmt-dev: run full tests on the latest GMT development version

Initial commit for adding the tilemap function for plotting XYZ tile maps. This is a wrapper around `pygmt.datasets.load_tile_map` and `pygmt.Figure.grdimage`. Aliases from `grdimage` have been copied over, and docstring for parameters from `load_tile_map` have been copied over too.
@weiji14 weiji14 added the feature Brand new feature label Mar 4, 2023
@weiji14 weiji14 added this to the 0.9.0 milestone Mar 4, 2023
@weiji14 weiji14 self-assigned this Mar 4, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Mar 4, 2023

Summary of changed images

This is an auto-generated report of images that have changed on the DVC remote

Status Path
added pygmt/tests/baseline/test_tilemap_no_clip_False.png
added pygmt/tests/baseline/test_tilemap_no_clip_True.png
added pygmt/tests/baseline/test_tilemap_ogc_wgs84.png
added pygmt/tests/baseline/test_tilemap_web_mercator.png

Image diff(s)

Added images

  • pygmt/tests/baseline/test_tilemap_no_clip_False.png

  • pygmt/tests/baseline/test_tilemap_no_clip_True.png

  • pygmt/tests/baseline/test_tilemap_ogc_wgs84.png

  • pygmt/tests/baseline/test_tilemap_web_mercator.png

Modified images

Path Old New

Report last updated at commit c3fca14

Let the Continuous Integration tests run with `rioxarray`, include it in pyproject.toml and environment.yml, and document it in `doc/install.rst` as an optional dependency.
doc/install.rst Outdated Show resolved Hide resolved
Comment on lines 33 to 45
# R="region",
V="verbose",
n="interpolation",
c="panel",
f="coltypes",
p="perspective",
t="transparency",
x="cores",
)
@kwargs_to_strings(c="sequence_comma", p="sequence") # R="sequence",
def tilemap(
self, region, zoom="auto", source=None, lonlat=True, wait=0, max_retries=2, **kwargs
):
Copy link
Member Author

Choose a reason for hiding this comment

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

Currently region needs to be a list to work with load_tile_map, but I'm debating on whether to allow strings like 0/1/2/3 or other styles supported by GMT -R as mentioned in #153 (comment) and https://docs.generic-mapping-tools.org/6.4/cookbook/options.html#data-domain-or-map-region-the-r-option

Copy link
Member

Choose a reason for hiding this comment

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

other styles supported by GMT -R as mentioned in #153 (comment) and https://docs.generic-mapping-tools.org/6.4/cookbook/options.html#data-domain-or-map-region-the-r-option

Other styles are more tricky to support. For example, how can we know the boundingbox for region="US"?

Copy link
Member Author

@weiji14 weiji14 Mar 4, 2023

Choose a reason for hiding this comment

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

Other styles are more tricky to support.

Hmm, I was mostly just thinking about 0/1/2/3, and maybe np.array([0, 1, 2, 3]). Usually users would set a region variable near the top of their code, and they might find it strange why their region that works for fig.basemap doesn't work for fig.tilemap.

For example, how can we know the boundingbox for region="US"?

If only there was an API in GMT to return the bounding box for a given region 😉 Global regions like region="g" and region="d" might be easy to support, but ISO code regions (https://www.pygmt.org/v0.8.0/tutorials/basics/regions.html#iso-code) like region="JP" and `region=JP+r3" will be a bit trickier.

Copy link
Member

Choose a reason for hiding this comment

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

If only there was an API in GMT to return the bounding box for a given region Global regions like region="g" and region="d" might be easy to support, but ISO code regions (https://www.pygmt.org/v0.8.0/tutorials/basics/regions.html#iso-code) like region="JP" and `region=JP+r3" will be a bit trickier.

Open a upstream feature request in the GMT repository?

Copy link
Member Author

@weiji14 weiji14 Mar 11, 2023

Choose a reason for hiding this comment

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

Hmm, or do you think we can use GMT_Extract_Region that's wrapped here:

pygmt/pygmt/clib/session.py

Lines 1500 to 1571 in 0b5a69b

def extract_region(self):
"""
Extract the WESN bounding box of the currently active figure.
Retrieves the information from the PostScript file, so it works for
country codes as well.
Returns
-------
* wesn : 1-D array
A numpy 1-D array with the west, east, south, and north dimensions
of the current figure.
Examples
--------
>>> import pygmt
>>> fig = pygmt.Figure()
>>> fig.coast(
... region=[0, 10, -20, -10],
... projection="M6i",
... frame=True,
... land="black",
... )
>>> with Session() as lib:
... wesn = lib.extract_region()
...
>>> print(", ".join([f"{x:.2f}" for x in wesn]))
0.00, 10.00, -20.00, -10.00
Using ISO country codes for the regions (for example ``'US.HI'`` for
Hawaii):
>>> fig = pygmt.Figure()
>>> fig.coast(
... region="US.HI", projection="M6i", frame=True, land="black"
... )
>>> with Session() as lib:
... wesn = lib.extract_region()
...
>>> print(", ".join([f"{x:.2f}" for x in wesn]))
-164.71, -154.81, 18.91, 23.58
The country codes can have an extra argument that rounds the region a
multiple of the argument (for example, ``'US.HI+r5'`` will round the
region to multiples of 5):
>>> fig = pygmt.Figure()
>>> fig.coast(
... region="US.HI+r5", projection="M6i", frame=True, land="black"
... )
>>> with Session() as lib:
... wesn = lib.extract_region()
...
>>> print(", ".join([f"{x:.2f}" for x in wesn]))
-165.00, -150.00, 15.00, 25.00
"""
c_extract_region = self.get_libgmt_func(
"GMT_Extract_Region",
argtypes=[ctp.c_void_p, ctp.c_char_p, ctp.POINTER(ctp.c_double)],
restype=ctp.c_int,
)
wesn = np.empty(4, dtype=np.float64)
wesn_pointer = wesn.ctypes.data_as(ctp.POINTER(ctp.c_double))
# The second argument to GMT_Extract_Region is a file pointer to a
# PostScript file. It's only valid in classic mode. Use None to get a
# NULL pointer instead.
status = c_extract_region(self.session_pointer, None, wesn_pointer)
if status != 0:
raise GMTCLibError("Failed to extract region from current figure.")
return wesn

Downside is that it requires a figure instance to be set up already first, before the bounding box coordinates can be extracted.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think it will work. Actually, a figure instance is not enough. We have to plot something to generate the hidden PostScript file before calling extract_region.

In [1]: import pygmt

In [2]: fig = pygmt.Figure()

In [3]: fig.region
pygmt-session [ERROR]: No hidden PS file found
---------------------------------------------------------------------------
GMTCLibError                              Traceback (most recent call last)
Cell In[3], line 1
----> 1 fig.region

File ~/OSS/gmt/pygmt/pygmt/figure.py:126, in Figure.region(self)
    124 self._activate_figure()
    125 with Session() as lib:
--> 126     wesn = lib.extract_region()
    127 return wesn

File ~/OSS/gmt/pygmt/pygmt/clib/session.py:1570, in Session.extract_region(self)
   1568 status = c_extract_region(self.session_pointer, None, wesn_pointer)
   1569 if status != 0:
-> 1570     raise GMTCLibError("Failed to extract region from current figure.")
   1571 return wesn

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it will work. Actually, a figure instance is not enough. We have to plot something to generate the hidden PostScript file before calling extract_region.

Hmm yeah, we would need to maybe do a fig.basemap and then fig.grdimage, but that's a bit of a hacky solution.

Let's just keep this PR simple and support only a list of [xmin, xmax, ymin, ymax] coordinates for now. There can be a follow up PR to support other region arguments (most likely after PyGMT v0.9.0) if there's interest in this 'enhancement'.

pygmt/src/tilemap.py Outdated Show resolved Hide resolved
Comment on lines +142 to +145
with Session() as lib:
lib.call_module(
module="grdimage", args=build_arg_string(kwargs, infile=tmpfile.name)
)
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that this is not the full implementation of PyGMT's fig.grdimage, which is actually a bit more complicated to support shading (-I) with an xarray.DataArray grid

pygmt/pygmt/src/grdimage.py

Lines 179 to 192 in 8f31706

with Session() as lib:
file_context = lib.virtualfile_from_data(check_kind="raster", data=grid)
with contextlib.ExitStack() as stack:
# shading using an xr.DataArray
if kwargs.get("I") is not None and data_kind(kwargs["I"]) == "grid":
shading_context = lib.virtualfile_from_data(
check_kind="raster", data=kwargs["I"]
)
kwargs["I"] = stack.enter_context(shading_context)
fname = stack.enter_context(file_context)
lib.call_module(
module="grdimage", args=build_arg_string(kwargs, infile=fname)
)

Question is: should we support shading of the XYZ tiles with xarray.DataArray grids? Or is there a way to use the grdimage implementation above directly without code duplication?

Copy link
Member

Choose a reason for hiding this comment

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

Currently I would say no to shading.

doc/install.rst Outdated Show resolved Hide resolved
Comment on lines 33 to 45
# R="region",
V="verbose",
n="interpolation",
c="panel",
f="coltypes",
p="perspective",
t="transparency",
x="cores",
)
@kwargs_to_strings(c="sequence_comma", p="sequence") # R="sequence",
def tilemap(
self, region, zoom="auto", source=None, lonlat=True, wait=0, max_retries=2, **kwargs
):
Copy link
Member

Choose a reason for hiding this comment

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

other styles supported by GMT -R as mentioned in #153 (comment) and https://docs.generic-mapping-tools.org/6.4/cookbook/options.html#data-domain-or-map-region-the-r-option

Other styles are more tricky to support. For example, how can we know the boundingbox for region="US"?

pygmt/src/tilemap.py Outdated Show resolved Hide resolved
pygmt/src/tilemap.py Outdated Show resolved Hide resolved
weiji14 and others added 5 commits March 4, 2023 21:21
Co-Authored-By: Dongdong Tian <[email protected]>
Need to go one more level down.

Co-Authored-By: Dongdong Tian <[email protected]>
Running the doctest with rioxarray installed with georeference the resulting `xarray.DataArray` and add a `spatial_ref` coordinate.
Just use a global map at zoom level 0 to make the test run in about 2s instead of >10s.
doc/api/index.rst Show resolved Hide resolved
Comment on lines +31 to +33
N="no_clip",
Q="nan_transparent",
# R="region",
Copy link
Member Author

@weiji14 weiji14 Mar 4, 2023

Choose a reason for hiding this comment

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

Was playing with the zoom parameter just now, and it seems that setting lower zoom levels (e.g. 0) will actually cause the area covered by contextily.bounds2img to be bigger than the original bounding box region. This might come as a surprise for users, so maybe we should set no_clip to False by default? Would require a bit of extra work to get correct. Edit: Sorry, got a bit confused by the double negatives, no_clip (-N) is already False by default, just needed to pass the region arguments to grdimage properly.

Alternatively, we could also do the clipping in pygmt.datasets.load_tile_map.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added commit 17d0734 to ensure that:

  • The plot is clipped to the bounding box region when no_clip=False (default)
  • The plot extends to areas outside of the bounding box region when no_clip=True

…pes, cores

Probably not needed with tilemap function.
Since `mamba` is recommended over `conda` as per #2385.
@seisman
Copy link
Member

seisman commented Mar 13, 2023

Usage:

import contextily
import pygmt

fig = pygmt.Figure()
fig.tilemap(
    region=[-157.6, -157.1, 1.68, 2.08],  # West, East, South, North
    source=contextily.providers.Stamen.Watercolor,
    frame=True,
)
fig.show()

produces this watercolor map of Kiribati:

Kiribati_watercolor_map

Do we need to convert the map tiles from EPSG:3857 to longitude/latitude?

pyproject.toml Outdated Show resolved Hide resolved
pygmt/src/tilemap.py Outdated Show resolved Hide resolved
pygmt/src/tilemap.py Outdated Show resolved Hide resolved
@maxrjones maxrjones mentioned this pull request Mar 14, 2023
36 tasks
weiji14 and others added 5 commits March 20, 2023 22:07
GIven a region in lonlat coordinates, ensure that the output figure is also plotted in lonlat instead of Web Mercator.
@weiji14
Copy link
Member Author

weiji14 commented Mar 20, 2023

Do we need to convert the map tiles from EPSG:3857 to longitude/latitude?

Yes, if lonlat=True (default), then I think the output map should be in lon/lat as well to make it intuitive. I've implemented this in fde94ad. The issue with this is that the pixels will be reprojected and distorted by default, which might not be a problem for tile maps like Stamen Terrain, but can be problematic when there are words on the tile map (e.g. for OpenStreetMap).

# bounding box region was provided in lonlat
if lonlat and raster.rio.crs == "EPSG:3857":
raster = raster.rio.reproject(dst_crs="OGC:CRS84")
raster.gmt.gtype = 1 # set to geographic type
Copy link
Member

Choose a reason for hiding this comment

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

Setting raster.gmt.gtype = 1 is useless, because raster.rio.to_raster doesn't understand it, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, rioxarray's .rio.to_raster won't understand .gmt.gtype, but I don't want to forget to add this after #2398 is merged 🙂

Comment on lines 145 to 148
# Only set region if no_clip is False, so that plot is clipped to exact
# bounding box region
if not kwargs.get("N"):
kwargs["R"] = "/".join(str(coordinate) for coordinate in region)
Copy link
Member

@seisman seisman Mar 20, 2023

Choose a reason for hiding this comment

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

When no_clip is not given, it defaults to None and not kwargs.get("N") is True, which means the plot is clipped. I find the code is a little difficult to understand. Do you think the following changes make it better? Or if kwargs.get("N") is in [None, False]?

Suggested change
# Only set region if no_clip is False, so that plot is clipped to exact
# bounding box region
if not kwargs.get("N"):
kwargs["R"] = "/".join(str(coordinate) for coordinate in region)
# Only set region if no_clip is False or None, so that plot is clipped to exact
# bounding box region
if kwargs.get("N") is not True:
kwargs["R"] = "/".join(str(coordinate) for coordinate in region)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, double negatives are confusing. I went with kwargs.get("N") in [None, False] at 125574e.

weiji14 and others added 2 commits March 23, 2023 21:10
Double negatives are confusing

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

@seisman seisman left a comment

Choose a reason for hiding this comment

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

This PR looks great to me.

@weiji14 weiji14 marked this pull request as ready for review March 23, 2023 09:07
@weiji14 weiji14 added final review call This PR requires final review and approval from a second reviewer and removed needs review This PR has higher priority and needs review. labels Mar 23, 2023
@seisman
Copy link
Member

seisman commented Mar 24, 2023

@michaelgrund and @yvonnefroehlich It would be great if you can give this PR a final review before we merge it.

Copy link
Member

@michaelgrund michaelgrund left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks again for working on that @weiji14 😉

Copy link
Member

@yvonnefroehlich yvonnefroehlich left a comment

Choose a reason for hiding this comment

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

I am afriad I do not have the knowledge to review this PR reliable, but what I understand make sense to me.

@seisman
Copy link
Member

seisman commented Mar 27, 2023

@weiji14 OK to merge?

@weiji14
Copy link
Member Author

weiji14 commented Mar 27, 2023

@weiji14 OK to merge?

Yes I'll merge it today. Let me incorporate #2394 (comment) first.

@weiji14 weiji14 enabled auto-merge (squash) March 27, 2023 01:18
@weiji14 weiji14 disabled auto-merge March 27, 2023 01:44
@weiji14
Copy link
Member Author

weiji14 commented Mar 27, 2023

Hold on, let me fix this dtype error on Windows / Python 3.11 + NumPy 1.24 at https://github.com/GenericMappingTools/pygmt/actions/runs/4527632070/jobs/7973665357?pr=2394#step:11:737. Looks similar to #2393:

_______________ [doctest] pygmt.datasets.tile_map.load_tile_map _______________
095     >>> from pygmt.datasets import load_tile_map
096     >>> raster = load_tile_map(
097     ...     region=[-180.0, 180.0, -90.0, 0.0],  # West, East, South, North
098     ...     zoom=1,  # less detailed zoom level
099     ...     source=contextily.providers.Stamen.TerrainBackground,
100     ...     lonlat=True,  # bounding box coordinates are longitude/latitude
101     ... )
102     >>> raster.sizes
103     Frozen({'band': 3, 'y': 256, 'x': 512})
104     >>> raster.coords
Differences (unified diff with -expected +actual):
    @@ -1,5 +1,5 @@
     Coordinates:
       * band         (band) uint8 0 1 2
    -  * y            (y) float64 -7.081e-10 -7.858e+04 ... -1.996e+07 ...
    +  * y            (y) float64 -7.081e-10 -7.858e+04 ... -1.996e+07 -2.004e+07
       * x            (x) float64 -2.004e+07 -1.996e+07 ... 1.996e+07 2.004e+07
    -    spatial_ref  int64 0
    +    spatial_ref  int32 0

D:\a\pygmt\pygmt\pygmt\datasets\tile_map.py:104: DocTestFailure
============================== warnings summary ===============================

So that the `spatial_ref` CF coordinate won't be set, which would result in an int32 variable on Windows but int64 on Linux/macOS.
@@ -147,6 +147,6 @@ def load_tile_map(region, zoom="auto", source=None, lonlat=True, wait=0, max_ret

# If rioxarray is installed, set the coordinate reference system
if hasattr(dataarray, "rio"):
dataarray = dataarray.rio.write_crs(input_crs="EPSG:3857")
dataarray = dataarray.rio.set_crs(input_crs="EPSG:3857")
Copy link
Member Author

Choose a reason for hiding this comment

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

Changed from using .rio.write_crs to .rio.set_crs to prevent the CF coordinate spatial_ref from being written to the output xarray.DataArray. This fixes the int32 on Windows bu int64 on Linux/macOS issue mentioned in #2394 (comment).

If this looks ok (and the tests pass), I'll proceed to merge in this PR.

Copy link
Member

Choose a reason for hiding this comment

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

Looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Brand new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integration with contextily to plot xyz basemaps
4 participants