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

Improve continuity between NE and OSM localities #2020

Closed
nvkelso opened this issue Dec 2, 2021 · 5 comments
Closed

Improve continuity between NE and OSM localities #2020

nvkelso opened this issue Dec 2, 2021 · 5 comments
Assignees
Milestone

Comments

@nvkelso
Copy link
Member

nvkelso commented Dec 2, 2021

When zooming across the NE <> OSM boundary around zoom 8, townspots shift position, population_rank values change, and min_zoom values change.

For country labels in the same places theme we do a join on WikidataID and apply the NE min_zoom to the OSM features.

We should do the same type of harvesting for localities. Namely:

  • OSM localities should get their population_rank and min_zoom values from NE.
    • The population value should not be adjusted, as that's for the incorporated area, while the NE population_rank is for the metro area and is more useful for label grading in the stylesheet.
  • NE localities might get their lat,lng from OSM? We'd need to do some QA to make sure the WikidataID joins are always for the right localities.

This is accomplished for countries with:

  • https://github.com/tilezen/vector-datasource/blob/v1.8.0/yaml/places.yaml#L100-L112
  • https://github.com/tilezen/vector-datasource/blob/024909ed8245a4ad4a25c908413ba3602de6c335/integration-test/977-min-zoom-from-ne-join.py
  • -- returns a JSONB object containing __ne_min_zoom and __ne_max_zoom set to the
    -- label min and max zoom of any matching row from the Natural Earth countries,
    -- map units and states/provinces themes.
    CREATE OR REPLACE FUNCTION tz_get_ne_min_max_zoom(wikidata_id TEXT)
    RETURNS JSONB AS $$
    DECLARE
    min_zoom REAL;
    max_zoom REAL;
    BEGIN
    IF wikidata_id IS NULL THEN
    RETURN '{}'::jsonb;
    END IF;
    -- first, try the countries table
    SELECT
    min_label, max_label INTO min_zoom, max_zoom
    FROM ne_10m_admin_0_countries c
    WHERE c.wikidataid = wikidata_id;
    -- if that fails, try map_units (which contains some sub-country but super-
    -- state level stuff such as England, Scotland and Wales).
    IF NOT FOUND THEN
    SELECT
    min_label, max_label INTO min_zoom, max_zoom
    FROM ne_10m_admin_0_map_units mu
    WHERE mu.wikidataid = wikidata_id;
    END IF;
    -- finally, try states and provinces
    IF NOT FOUND THEN
    SELECT
    min_label, max_label INTO min_zoom, max_zoom
    FROM ne_10m_admin_1_states_provinces sp
    WHERE sp.wikidataid = wikidata_id;
    END IF;
    -- return an empty JSONB rather than null, so that it can be safely
    -- concatenated with whatever other JSONB rather than needing a check for
    -- null.
    IF NOT FOUND THEN
    RETURN '{}'::jsonb;
    END IF;
    RETURN jsonb_build_object(
    '__ne_min_zoom', min_zoom,
    '__ne_max_zoom', max_zoom
    );
    END
    $$ LANGUAGE plpgsql STABLE;
  • def tags_set_ne_min_max_zoom(ctx):
    """
    Override the min zoom and max zoom properties with __ne_* variants from
    Natural Earth, if there are any.
    """
    params = _Params(ctx, 'tags_set_ne_min_max_zoom')
    layer_name = params.required('layer')
    layer = _find_layer(ctx.feature_layers, layer_name)
    for _, props, _ in layer['features']:
    min_zoom = props.pop('__ne_min_zoom', None)
    if min_zoom is not None:
    # don't overstuff features into tiles when they are in the
    # long tail of won't display, but make their min_zoom
    # consistent with when they actually show in tiles
    if min_zoom % 1 > 0.5:
    min_zoom = ceil(min_zoom)
    props['min_zoom'] = min_zoom
    elif props.get('kind') == 'country':
    # countries and regions which don't have a min zoom joined from NE
    # are probably either vandalism or unrecognised countries. either
    # way, we probably don't want to see them at zoom, which is lower
    # than most of the curated NE min zooms. see issue #1826 for more
    # information.
    props['min_zoom'] = max(6, props['min_zoom'])
    elif props.get('kind') == 'region':
    props['min_zoom'] = max(8, props['min_zoom'])
    max_zoom = props.pop('__ne_max_zoom', None)
    if max_zoom is not None:
    props['max_zoom'] = max_zoom
    return None
  • # IMPORTANT! do this _after_ the YAMLToDict default stuff, otherwise the
    # default will overwrite the value fron Natural Earth.
    - fn: vectordatasource.transform.tags_set_ne_min_max_zoom
    params:
    layer: places
@nvkelso nvkelso added this to the v1.9.0 milestone Dec 2, 2021
@nvkelso
Copy link
Member Author

nvkelso commented Dec 15, 2021

Should be paired with #1906 for POV point of view for locality kinds and country capital and region capital tags.

@nvkelso
Copy link
Member Author

nvkelso commented Jan 27, 2022 via email

@peitili
Copy link
Contributor

peitili commented Jan 27, 2022

Let’s skip the backfill.

On Wed, Jan 26, 2022 at 21:17 peitili @.> wrote: @nvkelso https://github.com/nvkelso I found that the proposal to use NE pop_min to backfill OSM population when this field is not available conflicts with this change #1993 <#1993>. If we want to keep both changes the default in 1993 and backfill logic here, it will be non-trivial, because we need to differentiate whether the population field is from the default or from the OSM in the transform function. Do we want to remove the change in 1993 or do we want to not do the backfill using pop_min? — Reply to this email directly, view it on GitHub <#2020 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGQIOZVVAVGSX43A3NDI2LUYDIOBANCNFSM5JGLWXYQ . You are receiving this because you were mentioned.Message ID: @.>

I actually later found a way to accommodate both the logic will be:

If the population is N/A, use NE pop_min, if NE pop_min is still unavailable use the estimate, I basically moved the estimate value from the yaml file to transform function. Let's wait for the integ tests to see how it goes. #2048

@nvkelso
Copy link
Member Author

nvkelso commented Jan 27, 2022

There's also a continuity problem where San Diego, California, drops between z7 & z7.999... because Natural Earth gives it a min_zoom of 4.0 but the OSM place=* and population=* logic gives it a min_zoom of 8.0.

This means that the min_zoom, population_rank, population sort that happens in grid thinning doesn't have enough consistent information to work with, and it can get omitted from tiles at that one zoom.

The solution is to also use the NE min_zoom for the OSM joined places.

@nvkelso
Copy link
Member Author

nvkelso commented Apr 29, 2022

Fixed via #2048

@nvkelso nvkelso closed this as completed Apr 29, 2022
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

2 participants