tilezen / vector-datasource

Tilezen vector tile service - OpenStreetMap data in several formats
https://www.nextzen.org/
Other
507 stars 119 forks source link

Move ne pop_max filter logic into min zoom #1369

Open rmarianski opened 7 years ago

rmarianski commented 7 years ago

In the ne populated places query, there are explicit checks for the population in the where clause. Can these be moved into the min zoom calculation instead?

nvkelso commented 7 years ago

Probably? They're there to ensure smaller regionally important cities from Natural Earth still show up when we switch over to OpenStreetMap for the first few zooms (which are just "big" cities).

Associated "remainder" tests are here:

rmarianski commented 7 years ago

Ok, I'm just coming from the perspective of trying to normalize the queries further. I'll give it a shot.

rmarianski commented 7 years ago

I started down this path, and began looking at updating the yaml for the min zoom calculation. It's a lookup table, and unless I"m missing a way to combine some of the operators we have, we'll need to make a new function and implement it in sql as well to contain this logic. @zerebubuth that sound right to you?

zerebubuth commented 7 years ago

Yup. Using lookup sounds good to me. I'm not sure what extra logic is needed to combine this with min_zoom - is there something in addition to taking the max of the lookup and whatever the previous min_zoom calculation was?

rmarianski commented 7 years ago

I was hoping for a quick win here, but it looks like to actually move this to yaml would require us to grow a max zoom concept. In particular, we need to have the ability for some features to selectively disappear at higher zoom levels based on population. We could just go with min zoom and filter out some of the data in a downstream transform, but I don't think that's the direction we want to go in.

nvkelso commented 7 years ago

Suggest tabling this for now.