Add GeoportailFrance tilelayers to providers.json.

[HaudinFlorence]

This PR proposes a new version of the json providers for the basemaps, including more than 250 additionnal tilelayers from Geoportail France. It also includes a piece of python code added to provider_sources/_compress_providers.py, used to generate the new providers from the xml requests. The names of the tilelayers have been automated from the original names and may be revisited if needed to reduce length/complexity.

[martinfleis]

Hi, thanks a lot for this massive expansion of the database.

To make it work within our current setup, the code that is currently in your generate_providers.py file should probably become a part of provider_sources/compress_providers.py. That way we always load fresh set from GeoportailFrance anytime we refresh the database. That is an automatised process composed of three steps right now - 1. fetch data from leaflet-providers, 2. merge them with additions specified in xyzservices-providers.json and 3. compress the result and save it as data/providers.json. I guess that if we add your code after the second step, the whole pipeline should work without any other changes. Apart from updating update_providers.yaml to include xmltodict and requests to make it run on GHA.

[HaudinFlorence]

Hi, thanks a lot for this massive expansion of the database.

To make it work within our current setup, the code that is currently in your generate_providers.py file should probably become a part of provider_sources/compress_providers.py. That way we always load fresh set from GeoportailFrance anytime we refresh the database. That is an automatised process composed of three steps right now - 1. fetch data from leaflet-providers, 2. merge them with additions specified in xyzservices-providers.json and 3. compress the result and save it as data/providers.json. I guess that if we add your code after the second step, the whole pipeline should work without any other changes. Apart from updating update_providers.yaml to include xmltodict and requests to make it run on GHA.

Hello. Thanks for the review. The suggestions proposed should have been taken into account in commit b9f4f5f.

[martinfleis]

Thanks! It looks great. I just compressed the JSON to include these in tests and committed that to check if all of these work as expected.

[HaudinFlorence]

Thanks! It looks great. I just compressed the JSON to include these in tests and committed that to check if all of these work as expected.

Ok. Thanks. Hoping everything will work correctly !

[martinfleis]

hmm, quite a few of these providers failed on CI (see https://github.com/geopandas/xyzservices/runs/8248560664?check_suite_focus=true). Can you have a look why is that? One reason may be that we use centre of bounds in the tile query

xyzservices/xyzservices/tests/test_providers.py

Lines 19 to 28 in 42e65c9

	def get_tile(provider):
	bounds = provider.get("bounds", [[-180, -90], [180, 90]])
	lat = (bounds[0][0] + bounds[1][0]) / 2
	lon = (bounds[0][1] + bounds[1][1]) / 2
	zoom = (provider.get("min_zoom", 0) + provider.get("max_zoom", 20)) // 2
	tile = mercantile.tile(lon, lat, zoom)
	z = tile.z
	x = tile.x
	y = tile.y
	return (z, x, y)

If some of these cover France and its overseas territories, I assume there is nothing in the centre. It is not the provider that is affected and we have this list of tiles where I always add one I know should be present whenever this occurs. Can you try to debug this and possibly add some known tiles to make CI happy?

xyzservices/xyzservices/tests/test_providers.py

Lines 63 to 71 in 42e65c9

	elif r == 404:
	# in some cases, the computed tile is not available. trying known tiles.
	options = [
	(12, 2154, 1363),
	(6, 13, 21),
	(16, 33149, 22973),
	(0, 0, 0),
	(2, 6, 7),
	]

[HaudinFlorence]

hmm, quite a few of these providers failed on CI (see https://github.com/geopandas/xyzservices/runs/8248560664?check_suite_focus=true). Can you have a look why is that? One reason may be that we use centre of bounds in the tile query

xyzservices/xyzservices/tests/test_providers.py

Lines 19 to 28 in 42e65c9

	def get_tile(provider):
	bounds = provider.get("bounds", [[-180, -90], [180, 90]])
	lat = (bounds[0][0] + bounds[1][0]) / 2
	lon = (bounds[0][1] + bounds[1][1]) / 2
	zoom = (provider.get("min_zoom", 0) + provider.get("max_zoom", 20)) // 2
	tile = mercantile.tile(lon, lat, zoom)
	z = tile.z
	x = tile.x
	y = tile.y
	return (z, x, y)

If some of these cover France and its overseas territories, I assume there is nothing in the centre. It is not the provider that is affected and we have this list of tiles where I always add one I know should be present whenever this occurs. Can you try to debug this and possibly add some known tiles to make CI happy?

xyzservices/xyzservices/tests/test_providers.py

Lines 63 to 71 in 42e65c9

	elif r == 404:
	# in some cases, the computed tile is not available. trying known tiles.
	options = [
	(12, 2154, 1363),
	(6, 13, 21),
	(16, 33149, 22973),
	(0, 0, 0),
	(2, 6, 7),
	]

I can have a look. I am not sure how to proceed (I am quite newbie with fixing CI test).

[HaudinFlorence]

I tried to add 2 tiles that are (if not mistaken) somewhere in the middle of France and another one in Paris. But it doesn't seem to solve the failing ci test issue.

[martinfleis]

I'll have a look either later today or in coming days.

[martinfleis]

We're down to 18 failures from initial 37! Good!

[HaudinFlorence]

We're down to 18 failures from initial 37! Good!

Yes. I am trying to reduce progressively the number of failures. Sorry for the multiple pushes.

[HaudinFlorence]

We're down to 18 failures from initial 37! Good!

I spent some time to try to get rid off the remaining failures but I couldn't, even when adding known tiles for which I can see something when displaying the tile layer with Leaflet. So I propose for the moment to exclude these 18 tile layers when producing the providers.json with _compress_providers.py? What do you think about it ? Or if you have any other idea, please let me know.

[martinfleis]

Let me have a look at it later this week/early next week.

[HaudinFlorence]

OK. I may miss something about the tests and adding tiles in the list of tiles known to work. Because here is an example with GeoportailFrance.Securoute_Te_All, that is part of the failing tests. I can see some something on the map (some dots ) when displaying the tile layer with leaflet in a js script, for instance on tile (9, 181, 259) as seen in the miniature.

But I have tried to add this tile in the list of known ones to work and it did not fix the failing test.

[martinfleis]

@HaudinFlorence I've added two other tiles and got it down to 9 layers returning 404. Not sure how to make those work, don't see them in leaflet either. So I marked them as broken. They are still included but our CI now doesn't fail when these don't resolve.

[HaudinFlorence]

OK. Thanks for the feedback and for reducing the number of failing layers.

[martinfleis]

Thanks a lot for this massive contribution ;).