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

TYP: Add type hints and improve docstrings of load_tile_map function #3087

Merged
merged 7 commits into from
Mar 10, 2024
Merged
Changes from 4 commits
Commits
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
103 changes: 46 additions & 57 deletions pygmt/datasets/tile_map.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,17 @@
:class:`xarray.DataArray`.
"""

from __future__ import annotations
from typing import Literal

from packaging.version import Version

try:
import contextily
from xyzservices import TileProvider

Check warning on line 12 in pygmt/datasets/tile_map.py

View check run for this annotation

Codecov / codecov/patch

pygmt/datasets/tile_map.py#L12

Added line #L12 was not covered by tests

_HAS_CONTEXTILY = True
except ImportError:
TileProvider = None
_HAS_CONTEXTILY = False

import numpy as np
Expand All @@ -21,76 +23,63 @@


def load_tile_map(
region,
zoom="auto",
source=None,
lonlat=True,
wait=0,
max_retries=2,
region: list,
zoom: int | Literal["auto"] = "auto",
source: TileProvider | str | None = None,
lonlat: bool = True,
wait: int = 0,
max_retries: int = 2,
zoom_adjust: int | None = None,
):
) -> xr.DataArray:
"""
Load a georeferenced raster tile map from XYZ tile providers.

The tiles that compose the map are merged and georeferenced into an
:class:`xarray.DataArray` image with 3 bands (RGB). Note that the returned
image is in a Spherical Mercator (EPSG:3857) coordinate reference system.
:class:`xarray.DataArray` image with 3 bands (RGB). Note that the returned image is
in a Spherical Mercator (EPSG:3857) coordinate reference system.

Parameters
----------
region : list
The bounding box of the map in the form of a list [*xmin*, *xmax*,
*ymin*, *ymax*]. These coordinates should be in longitude/latitude if
``lonlat=True`` or Spherical Mercator (EPSG:3857) if ``lonlat=False``.

zoom : int or str
Optional. Level of detail. Higher levels (e.g. ``22``) mean a zoom
level closer to the Earth's surface, with more tiles covering a smaller
geographical area and thus more detail. Lower levels (e.g. ``0``) mean
a zoom level further from the Earth's surface, with less tiles covering
a larger geographical area and thus less detail [Default is
``"auto"`` to automatically determine the zoom level based on the
bounding box region extent].
region
The bounding box of the map in the form of a list [*xmin*, *xmax*, *ymin*,
*ymax*]. These coordinates should be in longitude/latitude if ``lonlat=True`` or
Spherical Mercator (EPSG:3857) if ``lonlat=False``.
zoom
Level of detail. Higher levels (e.g. ``22``) mean a zoom level closer to the
Earth's surface, with more tiles covering a smaller geographical area and thus
more detail. Lower levels (e.g. ``0``) mean a zoom level further from the
Earth's surface, with less tiles covering a larger geographical area and thus
less detail. Default is ``"auto"`` to automatically determine the zoom level
based on the bounding box region extent.

.. note::
The maximum possible zoom level may be smaller than ``22``, and depends on
what is supported by the chosen web tile provider source.

source : xyzservices.TileProvider or str
Optional. The tile source: web tile provider or path to a local file.
Provide either:

- A web tile provider in the form of a
:class:`xyzservices.TileProvider` object. See
:doc:`Contextily providers <contextily:providers_deepdive>` for a
list of tile providers [Default is
``xyzservices.providers.OpenStreetMap.HOT``, i.e. OpenStreetMap
Humanitarian web tiles].
- A web tile provider in the form of a URL. The placeholders for the
XYZ in the URL need to be {x}, {y}, {z}, respectively. E.g.
source
The tile source: web tile provider or path to a local file. Provide either:

- A web tile provider in the form of a :class:`xyzservices.TileProvider` object.
See :doc:`Contextily providers <contextily:providers_deepdive>` for a list of
tile providers. Default is ``xyzservices.providers.OpenStreetMap.HOT``, i.e.
OpenStreetMap Humanitarian web tiles.
- A web tile provider in the form of a URL. The placeholders for the XYZ in the
URL need to be {x}, {y}, {z}, respectively. E.g.
``https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png``.
- A local file path. The file is read with
:doc:`rasterio <rasterio:index>` and all bands are loaded into the
basemap. See
- A local file path. The file is read with :doc:`rasterio <rasterio:index>` and
all bands are loaded into the basemap. See
:doc:`contextily:working_with_local_files`.

.. important::
Tiles are assumed to be in the Spherical Mercator projection (EPSG:3857).

lonlat : bool
Optional. If ``False``, coordinates in ``region`` are assumed to be
Spherical Mercator as opposed to longitude/latitude [Default is
``True``].

wait : int
Optional. If the tile API is rate-limited, the number of seconds to
wait between a failed request and the next try [Default is ``0``].

max_retries : int
Optional. Total number of rejected requests allowed before contextily
will stop trying to fetch more tiles from a rate-limited API [Default
is ``2``].

lonlat
If ``False``, coordinates in ``region`` are assumed to be Spherical Mercator as
opposed to longitude/latitude.
wait
If the tile API is rate-limited, the number of seconds to wait between a failed
request and the next try.
max_retries
Total number of rejected requests allowed before contextily will stop trying to
fetch more tiles from a rate-limited API.
zoom_adjust
The amount to adjust a chosen zoom level if it is chosen automatically. Values
outside of -1 to 1 are not recommended as they can lead to slow execution.
Expand All @@ -100,15 +89,15 @@

Returns
-------
raster : xarray.DataArray
raster
Georeferenced 3-D data array of RGB values.
Comment on lines 90 to 93
Copy link
Member

Choose a reason for hiding this comment

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

Now there are two sections, Return type and Returns, and the returned object is written in italic instead of bold style. I personally prefer the current documentation version, as I think the returned object and its data type should be stated together (analogous to the parameters).

current docs updated docs
tilemap_return_OLD tilemap_return_NEW

Copy link
Member Author

Choose a reason for hiding this comment

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

Need to check if sphinx-autodoc-typehint provides an option to control the behavior. https://github.com/tox-dev/sphinx-autodoc-typehints#options

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 sphinx-autodoc-typehint provides such an option, so there is little we can do unless we decide not to add type hints for the return values.

Copy link
Member

@weiji14 weiji14 Mar 8, 2024

Choose a reason for hiding this comment

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

There was a napoleon_use_rtype = True option that was tried in #937. Does it work here, or is that what sphinx-autodoc-typehint is using?

Copy link
Member

Choose a reason for hiding this comment

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

Can you try napoleon_preprocess_types = True as suggested at sphinx-doc/sphinx#9394?

Copy link
Member Author

Choose a reason for hiding this comment

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

napoleon_use_rtype = True incorrectly parses raster as the return type:
Screenshot from 2024-03-08 08-27-45

napoleon_preprocess_types = True and napoleon_use_rtype = False is not bad:

Screenshot from 2024-03-08 08-29-06

Copy link
Member Author

@seisman seisman Mar 8, 2024

Choose a reason for hiding this comment

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

https://www.sphinx-doc.org/en/master/usage/extensions/napoleon.html#confval-napoleon_preprocess_types

napoleon_preprocess_types
True to convert the type definitions in the docstrings as references. Defaults to False.

When setting napoleon_preprocess_types = True, raster is parsed as type definitions, so it's rendered the same way as the return type DataArray. Strictly speaking, it's still incorrect.

Actually, the name of the returned value should not appear in the NumPy-style docstrings (see https://www.sphinx-doc.org/en/master/usage/extensions/example_numpy.html#example-numpy). I.e., we should change

    Returns
    -------
    raster
        Georeferenced 3-D data array of RGB values.

to

    Returns
    -------
        Georeferenced 3-D data array of RGB values.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, the name of the returned value should not appear in the NumPy-style docstrings (see https://www.sphinx-doc.org/en/master/usage/extensions/example_numpy.html#example-numpy). I.e., we should change

It looks like they are using something like

    Returns
    -------
    xr.DataArray
        Georeferenced 3-D data array of RGB values.

Copy link
Member Author

Choose a reason for hiding this comment

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

That version doesn't have type hints for the return value.

Copy link
Member Author

Choose a reason for hiding this comment

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


Raises
------
ImportError
If ``contextily`` is not installed or can't be imported. Follow
seisman marked this conversation as resolved.
Show resolved Hide resolved
:doc:`install instructions for contextily <contextily:index>`, (e.g.
via ``python -m pip install contextily``) before using this function.
:doc:`install instructions for contextily <contextily:index>`, (e.g. via
``python -m pip install contextily``) before using this function.

Examples
--------
Expand Down
Loading