api icon indicating copy to clipboard operation
api copied to clipboard

assignLabels: improve duplicate label handling

Open Joxit opened this issue 5 years ago • 11 comments

This is the API part of pelias/labels#42, this will improve the user experience and developer experience.

Sometimes, we can see several identical labels, this is very annoying because we can not distinguish them. This was a long time issue for us @jawg.

Screenshot 2021-02-05 at 13 08 33

Our first response was, this is a integration issue, so update the name when you need to... So we implemented this in our demonstrator to say "hey, it's possible to do it on integration".

image

Now I realize it's awful for the developer experience... I blame myself terribly for not having thought of putting it in the API earlier... Especially since it's not complicated :sob:

So with this PR, Bagneux will return:

Bagneux, Hauts-de-Seine, France
Bagneux, Deux-Sèvres, France
Bagneux, Indre, France
Bagneux, Marne, France
Bagneux, Allier, France
Bagneux, Meurthe-et-Moselle, France
Bagneux, Aisne, France

And Sao Paulo will return:

São Paulo, SP, Brazil
São Paulo, AM, Brazil
São Paulo, MT, Brazil
São Paulo, MA, Brazil
São Paulo, BA, Brazil
São Paulo, ES, Brazil
São Paulo, PA, Brazil
Se, São Paulo, Brazil
Frei Paulo, Brazil
Sao Paulo, Brazil

Well known localities are still unchanged, for example Paris:

Paris, France
Paris, TX, USA
Paris, ON, Canada
Paris, TN, USA
Swainsboro, GA, USA
Paris Township, OH, USA
Paris Township, IL, USA
Paris Township, OH, USA
Paris, ME, USA
Paris, IA, USA

TODO: Change package.json before merge

related: pelias/labels#41 pelias/labels#42

Joxit avatar Feb 05 '21 11:02 Joxit

Yeah, this is great. We definitely should have built in some simple functionality to "extend" the labels before. And like you said, it's not actually complicated (at least not yet, I bet this system will grow 😇 )

orangejulius avatar Feb 05 '21 17:02 orangejulius

In the spirit of ways to make this more complicated in the future, an additional "extended" label feature could be the street of a POI. That would help with something like this for example :) image

orangejulius avatar Feb 05 '21 17:02 orangejulius

Hum, nice use case ! And I found an issue with my current PR :sweat_smile:

Police, Paris, France
Police nationale, Paris, Paris, France
Police Nationale, Paris, Paris, France
Police nationale, Paris, Paris, France
Police nationale, Paris, Paris, France
Police Nationale, Paris, Paris, France
Police nationale, Paris, Paris, France
Police Nationale, Paris, Paris, France
Police nationale, Paris, Paris, France
Police Nationale, Paris, Paris, France

Joxit avatar Feb 05 '21 19:02 Joxit

I drafted something for Starbucks :thinking: (streets only for venues with withOptional = true)

Starbucks, 5th Avenue, New York, NY, USA
Starbucks, 6th Avenue, New York, NY, USA
Starbucks, 3rd Avenue, New York, NY, USA
Starbucks, West 181st Street, New York, NY, USA
Starbucks, 7th Avenue, New York, NY, USA
Starbucks, 1st Avenue, New York, NY, USA
Starbucks, East 93rd Street, New York, NY, USA
Starbucks, East 90th Street, New York, NY, USA
Starbucks, West 145th Street, New York, NY, USA
Starbucks, West 23rd Street, New York, NY, USA

Joxit avatar Feb 05 '21 19:02 Joxit

This is huge from a user experience perspective, ❤️ it

missinglink avatar Feb 05 '21 21:02 missinglink

I agree ! :smile:

Joxit avatar Feb 05 '21 21:02 Joxit

PR rebase to master.

The CI failed because of Fastly outage :confused:

fetch http://dl-cdn.alpinelinux.org/alpine/v3.8/main/x86_64/APKINDEX.tar.gz
WARNING: Ignoring http://dl-cdn.alpinelinux.org/alpine/v3.8/main/x86_64/APKINDEX.tar.gz: temporary error (try again later)
fetch http://dl-cdn.alpinelinux.org/alpine/v3.8/community/x86_64/APKINDEX.tar.gz
WARNING: Ignoring http://dl-cdn.alpinelinux.org/alpine/v3.8/community/x86_64/APKINDEX.tar.gz: network error (check Internet connection and firewall)
ERROR: unsatisfiable constraints:
  bash (missing):
    required by: world[bash]
  curl (missing):
    required by: world[curl]

Exited with code exit status 2

Joxit avatar Jun 08 '21 10:06 Joxit

back to normal :+1:

Joxit avatar Jun 08 '21 11:06 Joxit

Updated and sync with #1565

Joxit avatar Oct 08 '21 09:10 Joxit

Thanks, I have reviewing/merging this on my TODO list.

missinglink avatar Oct 08 '21 12:10 missinglink

I've updated this PR to reflect changes from https://github.com/pelias/labels/pull/42

The interesting change here is this part: https://github.com/pelias/api/compare/1205687cf7ec73ac84ac699f4fa88db6f5fb2f31..fd1d0e5273a1439530ab1713a7b189bbbdd3263c

Now getBestLayers returns a Set instead of an array and filterUnambiguousParts returns a function (this will avoid the lint error Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (filterUnambiguousParts).

Joxit avatar May 19 '22 21:05 Joxit