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

_calculate_zoom returns negative values if coordinates in epsg:3857 are passed #193

Open
JacobThompsonFisher opened this issue Oct 17, 2021 · 3 comments

Comments

@JacobThompsonFisher
Copy link

This is an odd behavior that may not matter, and can possibly be closed immediately, but seemed worth documenting, as a user who just spent an hour debugging something.

If a user queries contextily.tile._calculate_zoom(w, s, e, n) using w,s,e,n from a 3857 shape, the function will return a negative value, presumably because it expects lon/lat inputs.

I would completely ignore this except that the core add_basemap function almost explicitly expects 3857.

If w,s,e,n points in epsg:4326 are passed to the _calculate_zoom function, it returns the expected values within nominal ranges for zoom.

Short test case, also attached as py in .txt
`import contextily as ctx

#total bounds box of a shape in epsg:3857
box3857 = [-13637706.360897053,
4527514.298503321,
-13631479.895919155,
4532126.494783246]

#total bounds box of a shape in epsg:4326
box4326 = [-122.503777375, 37.63160800000001, -122.459490625, 37.664412999999996]

#calc_zoom on the 3857 bounds
zoom3857 = ctx.tile._calculate_zoom(box3857[0], box3857[1], box3857[2], box3857[3])

#calc_zoom on the 4326 bounds
zoom4326 = ctx.tile._calculate_zoom(box4326[0], box4326[1], box4326[2], box4326[3])

print(f'EPSG:3857 result: {zoom3857} \n EPSG:4326 result: {zoom4326}')
Contextily_Test_Case.txt
`

@darribas
Copy link
Collaborator

Hello @JacobThompsonFisher,

Thanks very much for taking the interest in working with contextily! You're right, _calculate_zoom is probably not the most intuitive method in that it expects coordinates in lon/lat even though the zoom is really calculated on a Web Mercator context. The behaviour is documented however:

def _calculate_zoom(w, s, e, n):
"""Automatically choose a zoom level given a desired number of tiles.
.. note:: all values are interpreted as latitude / longitutde.
Parameters
----------
w : float
The western bbox edge.
s : float
The southern bbox edge.
e : float
The eastern bbox edge.
n : float
The northern bbox edge.
Returns
-------
zoom : int
The zoom level to use in order to download this number of tiles.
"""

And we signal the method is not necessarily intended for regular users by starting the function with the underscore (_).

In terms of going forward, perhaps one user improvement would be to come up with a user-facing method (calculate_zoom) that incorporated CRS switching functionality, so the default could be a bbox in every CRS, and that CRS. This is a bit how CRS management works in add_basemap, which is user-facing. What do you think?

@JacobThompsonFisher
Copy link
Author

I could not find that documentation to save my life, definitely a failure to rtfm.

For a user facing function I think you're absolutely right. I am not certain how many users actually need the function, but something which mirrors the bbox+CRS approach in add_basemap would be ideal. That said, probably a low priority given how rarely zoom customization on dynamic values seems to come up in the user base.

@martinfleis
Copy link
Member

We have actually discussed this already before in #172. I think we should create a public function that is more user friendly and flexible on input.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants