Adds zoom_adjust to add_basemap

[tristannew]

This PR seeks to resolve this issue #188. It is currently in progress and needs to be tested but if anyone sees this and has comments on my approach so far, any feedback would be appreciated 🙂 Also, there are some interim changes that will be removed, they are just there at the moment for testing, and since I was having some conda issues so it was quicker for me to create a local venv.

[martinfleis]

if anyone sees this and has comments on my approach so far, any feedback would be appreciated

looks reasonable from a quick skim

[tristannew]

@martinfleis Thank you for having a look! I am having one issue at the moment. With the changes, the code seems to get hung up on line 261 (256 without the changes).

contextily/contextily/tile.py

Lines 256 to 257 in e4de45b

	arrays = Parallel(n_jobs=n_connections, prefer=preferred_backend)(
	delayed(fetch_tile_fn)(tile_url, wait, max_retries) for tile_url in tile_urls)

If I don't pass zoom_adjust to add_basemap it works as expected, if I do pass it as an argument it is either very slow or keeps running until the kernel times out.

I can't think why adding an integer value to zoom would affect anything else, but I must surely be missing something. Any ideas?

[martinfleis]

If the adjustment is larger than 1 or 2 zoom levels, you need to download a lot more tiles than before. That may take some time. But I can have a look at the code if you think it is something in there.

[tristannew]

Ah that makes sense! It is taking a while on my machine so I think I'll spend some time trying to see if it can be sped up at all. Otherwise, from a user perspective, my implementation of zoom_adjust becomes a bit difficult to use

[martinfleis]

This is useful to adjust the zoom level by ±1. Nothing else makes much sense. And for these cases, it works as expected (just tested). It also works with 2, though it takes a bit. And it is exactly the same as implemented in Place. So I would go ahead with this and add a note to the docstring instructing users that anything else than 1 or -1 is not recommended.

This exists because sometimes the data extent is just on the verge of two zoom levels and you may prefer the other one that was selected automatically.

[tristannew]

Perfect, that is all done then I think. My most recent commit just includes the changes we have been discussing and an additional note in the docstrings. Is there a changelog that I should add to? Thank you so much for the help and guidance here!

[martinfleis]

We use Github release notes as a changelog so no need to add anything now.

What would be nice, though, is a test. Can you add some small test, checking if the keyword works as intended? No need for anything special but we should ensure that we're loading higher or lower number of tiles than by default.

[tristannew]

Sure thing, I can do that 🙂

[tristannew]

Sorry it has taken me a while to get back to this - I added the test a while ago but the changes seem to violate the test_add_basemape_query test. The values seem to have changed slightly, so I have edited them to get the tests passing. But I wouldn't be able to say whether that is an destructive change that I have made. If it isn't, then should be good to go!

[martinfleis]

Thanks. lot!