-
Notifications
You must be signed in to change notification settings - Fork 220
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 zoom_adjust parameter to pygmt.datasets.load_tile_map and Figure.tilemap #2934
Conversation
The zoom_adjust parameter is used to adjust the automatic zoom level by integer increments. This was added in contextily=1.5.0, see geopandas/contextily#228.
pygmt/datasets/tile_map.py
Outdated
@@ -128,6 +141,7 @@ def load_tile_map(region, zoom="auto", source=None, lonlat=True, wait=0, max_ret | |||
ll=lonlat, | |||
wait=wait, | |||
max_retries=max_retries, | |||
zoom_adjust=zoom_adjust, |
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.
Might need to make this backwards compatible with contextily<1.5.0
.
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.
Yes, need to check the contextily version and also better to mention that the zoom_adjust
parameter requires contextily>=1.5.0.
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.
Also mention that the `zoom_adjust` parameter was added in PyGMT v0.11.0 using the versionadded sphinx directive (https://www.sphinx-doc.org/en/master/usage/restructuredtext/directives.html#directive-versionadded).
pygmt/datasets/tile_map.py
Outdated
lonlat=True, | ||
wait=0, | ||
max_retries=2, | ||
zoom_adjust=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.
Add type hints to this newly added parameter?
zoom_adjust=None, | |
zoom_adjust: int | 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.
Done at f0c95d8 for just the zoom_adjust
parameter. Since all the lines are changing, should I just add type hints for all the other parameters too?
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.
Perfer to focus on the zoom_adjust
parameter only in this PR.
pygmt/datasets/tile_map.py
Outdated
automatically. Values outside of -1 to 1 are not recommended as they | ||
can lead to slow execution. [Default is ``None``]. | ||
|
||
.. versionadded:: 0.11.0 |
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 we should not add this until PyGMT reaches v1.0.0?
.. versionadded:: 0.11.0
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.
Fixes `error: Argument 1 to "unsignedinteger" has incompatible type "list[int]"; expected "SupportsInt | str | bytes | SupportsIndex" [arg-type]`
Co-Authored-By: Dongdong Tian <[email protected]>
@@ -117,6 +135,15 @@ def load_tile_map(region, zoom="auto", source=None, lonlat=True, wait=0, max_ret | |||
"to install the package." | |||
) | |||
|
|||
contextily_kwargs = {} |
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.
Do you want to also include zoom
, source
, ll
, wait
and max_retries
in contextily_kwargs
?
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.
Mm, not really necessary. I only made the contextily_kwargs
dict to make things backwards compatible with contextily<1.5.0
.
Gonna leave this open for another day or so in case someone wants to comment on anything. |
Co-authored-by: Yvonne Fröhlich <[email protected]>
Co-authored-by: Dongdong Tian <[email protected]>
Description of proposed changes
The
zoom_adjust
parameter is used to adjust the automatic zoom level by integer increments.Preview at https://pygmt-dev--2934.org.readthedocs.build/en/2934/api/generated/pygmt.datasets.load_tile_map.html
Note: This requires
contextily>=1.5.0
, see geopandas/contextily#228.Example usage:
produces
Fixes #
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 commands are:
/format
: automatically format and lint the code/test-gmt-dev
: run full tests on the latest GMT development version