-
Notifications
You must be signed in to change notification settings - Fork 224
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
load_tile_map: Add the new parameter 'crs' to set the CRS of the returned dataarray #3554
Conversation
c34a56f
to
d79af32
Compare
259b0dd
to
a48f9df
Compare
a48f9df
to
00e2605
Compare
""" | ||
kwargs = self._preprocess(**kwargs) | ||
|
||
if not _HAS_RIOXARRAY: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Figure.tilemap
method no longer requires rioxarray
explicitly. Instead, load_tile_map
will raise the error.
raster = load_tile_map( | ||
region=region, | ||
zoom=zoom, | ||
source=source, | ||
lonlat=lonlat, | ||
crs="OGC:CRS84" if lonlat is True else "EPSG:3857", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If lonlat=True
, set the CRS to let load_tile_map
do the raster reprojection for us.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
from rasterio.crs import CRS | ||
from xyzservices import TileProvider | ||
|
||
_HAS_CONTEXTILY = True | ||
except ImportError: | ||
CRS = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe put the rasterio
imports in the rioxarray block below instead of here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rasterio
is a known dependency of contextily
(xref: https://github.com/geopandas/contextily/blob/main/pyproject.toml), so it makes more sense to assume that they're installed together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rasterio is also a dependency of rioxarray
😆. I would argue that rasterio
is more tightly coupled to rioxarray
, and since rasterio
/rioxarray
are both used for their projection system capabilities, it would make more sense to put those together. But ok too if you want to keep it here.
pygmt/datasets/tile_map.py
Outdated
@@ -128,6 +138,8 @@ def load_tile_map( | |||
... raster.rio.crs.to_string() | |||
'EPSG:3857' | |||
""" | |||
_default_crs = "EPSG:3857" # The default CRS returned from contextily |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At https://contextily.readthedocs.io/en/latest/reference.html#contextily.bounds2img, there is this sentence:
IMPORTANT: tiles are assumed to be in the Spherical Mercator projection (EPSG:3857), unless the crs keyword is specified.
While tilemaps are usually served in EPSG:3857, I know that there are some tilemaps that can be served in a different projection system by default. I'm hesitant to use this default of EPSG:3857 here, unless we can be confident that contextily only supports EPSG:3857 tiles.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the contexily source codes, I think contexitly always assumes that CRS is EPSG:3857, at least when writing the image to raster files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is for the warp_tiles
function. I don't see anything about EPSG:3857 in the bounds2img
function (though it's calling several functions and I haven't checked too closely).
Non-Web Mercator (EPSG:3857) XYZ tiles are not common, but they do exist, e.g. Openlayers allows configuring the projection system of the returned tiles:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the contexily source codes, I think contexitly always assumes that CRS is EPSG:3857, at least when writing the image to raster files
I meant this line:
In bounds2raster, it first calls bounds2img
to get the image
and extent
(can be any CRS), but when writing to a raster image, bounds2raster
assumes the image
is EPSG:3857
.
Looking at the xyzservices (https://github.com/geopandas/xyzservices/blob/c847f312d2e67a0a45ceae119fdcdf7cf60858b6/provider_sources/xyzservices-providers.json#L50), I can also see some providers that has the crs
attribute, but it's unclear how the crs
attribute is handled in contexitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried using xyzservices.providers.MapTiler.Basic4326
which comes in EPSG:4326:
import contextily
import xyzservices
import matplotlib.pyplot as plt
source = xyzservices.providers.MapTiler.Basic4326(
key="BwTZdryxoypHc4BnCZ4" # add an 'o' to the end
)
img, bounds = contextily.bounds2img(w=0, s=-80, e=150, n=80, ll=True, source=source)
# img.shape # (4096, 2048, 4)
plt.imshow(img[:, :, :3])
produces
The extent doesn't seem to be correct (it should be plotting from Greenwich to Asia), possibly a bug in contextily
hardcoding to EPSG:3857. But it does show that non-Web Mercator XYZ tiles are indeed possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found geopandas/contextily#119, and had a closer look at the bounds2img
code, specifically these lines - https://github.com/geopandas/contextily/blob/00ca0318033e69e29a164ce571d9c932a057ecc6/contextily/tile.py#L270-L272. It seems like contextily
is using mercantile
which only supports Spherical Mercator (EPSG:3857), see mapbox/mercantile#109 (comment). Unless contextily
switches to something like https://github.com/developmentseed/morecantile in the future, then it would only 'work' with EPSG:3857.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The extent doesn't seem to be correct (it should be plotting from Greenwich to Asia), possibly a bug in
contextily
hardcoding to EPSG:3857.
When ll=True
, bounds2img
calls the private _sm2ll
function to convert bounds in Spherical Mercator coordinates to lon/lat. I believe "EPSG:3857" is hardcoded/assumed in many places in the contextily package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about checking source.crs
(an attribute from an instance of the xyzservices.TileProvider
class), and if it is present, use that as the default crs. If not, fallback to EPSG:3857.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. Done in 6733e19.
_default_crs = getattr(source, "crs", "EPSG:3857")
Please note that this line of code works for TileProvide
, str
and None
. str
and None
don't have crs
, so EPSG:3857
is returned.
Edit: renamed _default_crs
to _source_crs
in c1d55b0.
Please note that, currently, the returned xarray.DataArray is still "EPSG:3857" even if the source CRS is not (i.e. we don't the reprojection). This may not be ideal.
pygmt/datasets/tile_map.py
Outdated
@@ -128,6 +138,8 @@ def load_tile_map( | |||
... raster.rio.crs.to_string() | |||
'EPSG:3857' | |||
""" | |||
_default_crs = "EPSG:3857" # The default CRS returned from contextily |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is for the warp_tiles
function. I don't see anything about EPSG:3857 in the bounds2img
function (though it's calling several functions and I haven't checked too closely).
Non-Web Mercator (EPSG:3857) XYZ tiles are not common, but they do exist, e.g. Openlayers allows configuring the projection system of the returned tiles:
…rned dataarray (#3554) Co-authored-by: Wei Ji <[email protected]>
Description of proposed changes
This PR adds the
crs
parameter to theload_tile_map
function so that users can change the CRS of the returned raster image without needing to know the details.Address #2125 (comment).
Closes #3484.
Notes for maintainers:
As far as I know, there are multiple different ways to reproject raster images.
rasterio.vrt.WarpedVRT
rasterio.warp.reproject
rasterio.vrt.WarpedVRT
rasterio.warp.reproject
gdal.Warp
Options 3-5 are complicated and I don't think we want to call them directly. Ideally, we should use option 1, so reprojection doesn't rely on an extra dependency
rioxarray
. However, as shown below, the result fromcontextily.warp_tiles
doesn't work well withrio.to_raster
.Here are codes to compare the results with options 1 and 2:
The outputs are:
As you can see, the longitude range is (-180.55157789333614, 180.18036434850873) with
contextily.warp_tiles
, which will lead to GMT errors like:Two more notes:
rasterio.warp.reproject
instead ofrasterio.vrt.WarpedVRT
inwarp_tiles
(Explorerasterio.warp.reproject
to replaceMemoryFile
for warping geopandas/contextily#88). If it's done, then the result fromwarp_tiles
will be likely the same as the one fromrio.reproject
rio.to_raster
(pygmt/pygmt/helpers/tempfile.py
Line 204 in 988746b
Preview: https://pygmt-dev--3554.org.readthedocs.build/en/3554/api/generated/pygmt.datasets.load_tile_map.html
Reminders
make format
andmake check
to make sure the code follows the style guide.doc/api/index.rst
.Slash Commands
You can write slash commands (
/command
) in the first line of a comment to performspecific operations. Supported slash command is:
/format
: automatically format and lint the code