pelias / api

HTTP API for Pelias Geocoder
http://pelias.io
MIT License
221 stars 162 forks source link

autocomplete: add macrocounty to list of potential admin matching fields #1552

Closed missinglink closed 5 months ago

missinglink commented 3 years ago

The query "2 Macquarie Street, Sydney" does not currently return either of these two results despite Parramatta being a suburb within the greater Metro Sydney Area.

The reason for this is that the Metro Area is rather large and so WOF has designated it as a 'macrocounty'.

We're currently not targeting macrocounty in the list of fields which are queried for the 'admin portion' of an autocomplete query:

Screenshot 2021-08-12 at 11 04 39

With this PR we add macrocounty to the existing list:

Screenshot 2021-08-12 at 10 59 36

The macrocounty placetype is seldom used, it only has 477 places at time of writing, so this likely won't have any adverse effects.

@Joxit I noticed on that link above that there's a bunch in France, would this change be beneficial/detrimental for you?

missinglink commented 3 years ago

wow, so actually, that's apparently a very common street name in Sydney, how confusing 😆 https://pelias.github.io/compare/#/v1/autocomplete?boundary.gid=whosonfirst%3Amacrocounty%3A1376953385&text=2+Macquarie+Street

Joxit commented 3 years ago

Hum, in France, the macrocounty is a subdivision of the region where the name of the macrocounty is most often the name of capital/main city... It's not common to use macrocounty in daily life.

I tried something, a street named Avenue Aristide Briand which crosses several cities in the macrocounty named Antony. :memo: Antony, Bagneux, Montrouge, Bourg-la-Reine, Le Plessis-Robinson are cities that are part of the Antony macrocounty.

Before the PR

Avenue Aristide Briand, Antony

Avenue Aristide Briand, Antony, France

After the PR

Avenue Aristide Briand, Antony

Avenue Aristide Briand, Antony, France
Avenue Aristide Briand (Coté Bagneux), Bagneux, France
Avenue Aristide Briand, Montrouge, France
Avenue Aristide Briand, Bagneux, France
Avenue Aristide Briand, Bourg-la-Reine, France
Avenue Aristide Briand, Le Plessis-Robinson, France

The new result is a bit weird but Antony city is still in first position then...

Same behavior with venues

Credit Agricole, Antony

Crédit Agricole, Antony, France
Crédit Agricole, Montrouge, France
Crédit Agricole, Clamart, France
Crédit Agricole, Châtillon, France
Crédit Agricole, Vanves, France
Crédit Agricole, Le Plessis-Robinson, France
Crédit Agricole, Bourg-la-Reine, France
Crédit Agricole, Sceaux, France
Crédit Agricole - Forum, Montrouge, France
Crédit Agricole immobilier, Montrouge, France
missinglink commented 3 years ago

Hey @Joxit I'm feeling less confident with merging this now, I like the improvements in Australia but it feels like it might come with a regression in France/Germany and any other countries which use macrocounty.

I see you approved the PR, does that mean you think it's still worth merging despite that?

Joxit commented 3 years ago

Hi @missinglink, results for France reminded me this draft pelias/wof-admin-lookup#300

IMO, if France were the only country affected and if it improves the results in Australia, I think it may be worth it. Since we are talking about autocomplete, I think that returning results from nearby cities might not be a bad idea either :thinking: (even if in my examples there are too many results :sweat_smile:)

missinglink commented 3 years ago

🤔 thinking... {buffering}

missinglink commented 5 months ago

This PR stalled due to it increasing noise in France while also fixing issues in Australia.

It seems that the issues in Australia have worsened now since some WOF refactoring and many addresses are no longer retrievable in Melbourne (for instance) because it only appears as a macroregion in the document parents.

I feel that the omission of macroregion from the query list was due to oversight (or it being introduced later) rather than intent, we should be listing all the parent fields in the query.

For that reason I'd like to imminently merge this PR.