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

Switch away from Stamen basemaps #2717

Merged
merged 2 commits into from
Oct 5, 2023
Merged

Switch away from Stamen basemaps #2717

merged 2 commits into from
Oct 5, 2023

Conversation

weiji14
Copy link
Member

@weiji14 weiji14 commented Oct 4, 2023

Description of proposed changes

Stamen basemaps are being deprecated, so switching the default source to OpenStreetMap Humanitarian web tiles following contextily at geopandas/contextily#221. Changed load_tile_map's inline example to use OpenTopoMap, and tilemap gallery example to use CartoDB's Positron basemap instead.

Preview at https://pygmt-dev--2717.org.readthedocs.build/en/2717/gallery/maps/tilemaps.html

Fixes #2716

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

Stamen basemaps are being deprecated, so switching the default source to OpenStreetMap Humanitarian web tiles following contextily. Changed load_tile_map's inline example to use OpenTopoMap, and tilemap gallery example to use USGS's USImageryTopo instead.
@weiji14 weiji14 added the bug Something isn't working label Oct 4, 2023
@weiji14 weiji14 self-assigned this Oct 4, 2023
@weiji14 weiji14 marked this pull request as ready for review October 4, 2023 20:28
Comment on lines 40 to 41
# Use the U.S. Geological Survey Imagery Topo web tiles from contextily
source=contextily.providers.USGS.USImageryTopo,
Copy link
Member Author

@weiji14 weiji14 Oct 4, 2023

Choose a reason for hiding this comment

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

The USImageryTopo map looks like this:

image

Alternatively, I was also thinking that the CartoDB basemaps could be a good option (that is also global in coverage). E.g. CartoDB Positron which has a lighter colour and might be better to overlay other data on top:

cartodb_positron

Any preference, or other options to consider?

Copy link
Member

Choose a reason for hiding this comment

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

I was also thinking that the CartoDB basemaps could be a good option (that is also global in coverage). E.g. CartoDB Positron which has a lighter colour and might be better to overlay other data on top:

Sounds good to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, switched to CartoDB Positron in 996f898.

@weiji14
Copy link
Member Author

weiji14 commented Oct 4, 2023

Noticing that tilemap is also breaking with GMT 6.5.0_5496e05_2023.10.05 dev version, since GenericMappingTools/gmt#7827 was merged I think 😅. Traceback from https://github.com/GenericMappingTools/pygmt/actions/runs/6411155128/job/17405984685#step:16:2360:

__________________________ test_tilemap_web_mercator ___________________________

args = (), kwargs = {}

    def wrapper(*args, **kwargs):
>       store.return_value[test_name] = obj(*args, **kwargs)

../../../../micromamba/envs/pygmt/lib/python3.11/site-packages/pytest_mpl/plugin.py:107: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
../pygmt/tests/test_tilemap.py:17: in test_tilemap_web_mercator
    fig.tilemap(
../pygmt/helpers/decorators.py:598: in new_module
    return module_func(*args, **kwargs)
../pygmt/helpers/decorators.py:738: in new_module
    return module_func(*args, **kwargs)
../pygmt/src/tilemap.py:148: in tilemap
    lib.call_module(
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <pygmt.clib.session.Session object at 0x7f877c8b63d0>
module = 'grdimage'
args = '/tmp/pygmt-ih40ojsv.tif -Bafg -R-20000000.0/20000000.0/-20000000.0/20000000.0'

    def call_module(self, module, args):
        """
        Call a GMT module with the given arguments.
    
        Makes a call to ``GMT_Call_Module`` from the C API using mode
        ``GMT_MODULE_CMD`` (arguments passed as a single string).
    
        Most interactions with the C API are done through this function.
    
        Parameters
        ----------
        module : str
            Module name (``'coast'``, ``'basemap'``, etc).
        args : str
            String with the command line arguments that will be passed to the
            module (for example, ``'-R0/5/0/10 -JM'``).
    
        Raises
        ------
        GMTCLibError
            If the returned status code of the function is non-zero.
        """
        c_call_module = self.get_libgmt_func(
            "GMT_Call_Module",
            argtypes=[ctp.c_void_p, ctp.c_char_p, ctp.c_int, ctp.c_void_p],
            restype=ctp.c_int,
        )
    
        mode = self["GMT_MODULE_CMD"]
        status = c_call_module(
            self.session_pointer, module.encode(), mode, args.encode()
        )
        if status != 0:
>           raise GMTCLibError(
                f"Module '{module}' failed with status code {status}:\n{self._error_message}"
            )
E           pygmt.exceptions.GMTCLibError: Module 'grdimage' failed with status code 79:
Error:      grdimage [ERROR]: Byte image without indexed color requires a CPT via -C

../pygmt/clib/session.py:628: GMTCLibError
----------------------------- Captured stderr call -----------------------------
Error: e [ERROR]: Byte image without indexed color requires a CPT via -C

Switch from USGS Imagery Topo basemap to CartoDB Positron that is lighter in colour and suited for overlaying point data on top.
@seisman
Copy link
Member

seisman commented Oct 5, 2023

Change the PR title to something like?

Figure.tilemap: Change the default tile provider to OpenStreetMap Humanitarian"

@weiji14
Copy link
Member Author

weiji14 commented Oct 5, 2023

Change the PR title to something like?

Figure.tilemap: Change the default tile provider to OpenStreetMap Humanitarian"

Mm, but there are also some changes to pygmt.datasets.load_tile_map, and also not every basemap source was changed to OpenStreetMap Humanitarian 😅

@seisman
Copy link
Member

seisman commented Oct 5, 2023

Mm, but there are also some changes to pygmt.datasets.load_tile_map,

Then maybe Figure.tilemap & pygmt.datasets.load_tile_map: Change the default tile provider to OpenStreetMap Humanitarian"?

The main idea is to use the PR title in the changelog so that it's easy to know the "breaking" changes in the future.

@weiji14
Copy link
Member Author

weiji14 commented Oct 5, 2023

The default tile provider actually depends on what version of contextily users have installed (i.e. Stamen for contextily<1.4.0, OpenStreetMap HOT for contextily>=1.4.0). We're not setting any default in PyGMT, only changing the documentation to match contextily upstream. Maybe we should label this as a documentation change instead?

@seisman
Copy link
Member

seisman commented Oct 5, 2023

The default tile provider actually depends on what version of contextily users have installed (i.e. Stamen for contextily<1.4.0, OpenStreetMap HOT for contextily>=1.4.0).

I see.

We're not setting any default in PyGMT, only changing the documentation to match contextily upstream. Maybe we should label this as a documentation change instead?

Sounds good. Then then PR title doesn't matter much now.

@weiji14 weiji14 added documentation Improvements or additions to documentation and removed bug Something isn't working labels Oct 5, 2023
@weiji14
Copy link
Member Author

weiji14 commented Oct 5, 2023

Ok, I'll merge this in then.

@weiji14 weiji14 merged commit 4d289b2 into main Oct 5, 2023
14 of 15 checks passed
@weiji14 weiji14 deleted the switch-stamen-basemaps branch October 5, 2023 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docs suddenly break due to contextily?
2 participants