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

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

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

missinglink
Copy link
Member

@missinglink missinglink commented Aug 12, 2021

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
Copy link
Member Author

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
Copy link
Member

Joxit commented Aug 12, 2021

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.
📝 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
Copy link
Member Author

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
Copy link
Member

Joxit commented Aug 19, 2021

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 🤔 (even if in my examples there are too many results 😅)

@missinglink
Copy link
Member Author

missinglink commented Aug 20, 2021

🤔 thinking... {buffering}

@missinglink
Copy link
Member Author

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.

@missinglink missinglink merged commit aa458a1 into master Mar 28, 2024
7 checks passed
@missinglink missinglink deleted the macrocounty branch March 28, 2024 14:04
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

Successfully merging this pull request may close these issues.

None yet

3 participants