-
-
Notifications
You must be signed in to change notification settings - Fork 163
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
Conversation
wow, so actually, that's apparently a very common street name in Sydney, how confusing 😆 |
Hum, in France, the I tried something, a street named Before the PR
After the PR
The new result is a bit weird but Antony city is still in first position then... Same behavior with venues
|
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 I see you approved the PR, does that mean you think it's still worth merging despite that? |
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. |
🤔 thinking... {buffering} |
fef6183
to
4ab6b13
Compare
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 I feel that the omission of For that reason I'd like to imminently merge this PR. |
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:With this PR we add
macrocounty
to the existing list: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?