Fix: Enable qualifier searches for unnamed POIs near small places#4035
Fix: Enable qualifier searches for unnamed POIs near small places#4035agam1092005 wants to merge 1 commit intoosm-search:masterfrom
Conversation
|
@lonvia |
lonvia
left a comment
lonvia
left a comment
There was a problem hiding this comment.
Thanks for trying this out. The direction this is going is already good. However, given that we'd need special treatment for both PlaceSearch (restrict to places) and NearSearch (only use if there is a single result), it might be better to just go with our own adapted Search type.
|
|
||
| if len(new_results) > 1: | ||
| for result in new_results: | ||
| result.accuracy += 0.2 |
There was a problem hiding this comment.
You can't add a general penalty hear because near search is also used for actual near searches ('pub in kingston') where multiple results are perfectly okay.
| bool(near_items)) | ||
| else: | ||
| if (sdata.qualifiers and not near_items and | ||
| len(sdata.qualifiers.values) == 1 and |
There was a problem hiding this comment.
This condition is actually prevents 'kingston pub' from working when using the wiki special phrases set. The word pub is mapped to two qualifiers: amenity=bar and amenity=pub. That is not unusual and shouldn't really be a problem here.
| if (sdata.qualifiers and not near_items and | ||
| len(sdata.qualifiers.values) == 1 and | ||
| not self.details.categories): | ||
| import copy |
There was a problem hiding this comment.
imports should always go to the top of the file.
| not self.details.categories): | ||
| import copy | ||
| sdata_near = copy.copy(sdata) | ||
| sdata_near.qualifiers = dbf.WeightedCategories([], []) |
There was a problem hiding this comment.
You overwrite the qualifiers here only to hand them in again in the function below.
It's probably better to simply hand in the original sdata, make temporary modifications in place and then restore the original values before returning. Not ideal, I know, but at least it keeps the related code together.
|
|
||
| near_builder = self._build_qualifier_address_search( | ||
| sdata_near, sdata.qualifiers, | ||
| [assignment.name] + assignment.address, |
There was a problem hiding this comment.
Hand those two parameters in separately instead of concatenating. They are just separated to their original state in the function.
| """ | ||
| penalty = min(categories.penalties) | ||
| categories.penalties = [p - penalty for p in categories.penalties] | ||
| for search in self.build_name_search(sdata, address[0], address[1:], |
There was a problem hiding this comment.
You can't really use the name builder here because it creates address and place searches. We'd be only interested in village names, so place search es only.
And this brings up another problem: the place search would need to be restricted to ranks that describe small villages/suburbs etc. PlaceSearch currently cannot do that. It only gets such restrictions from the details parameter that is injected internally.
|
Thanks for the detailed review! I'm addressing all the feedback:
Regarding the rank restriction for small places: Happy to explore a different approach if you'd prefer the restriction to happen earlier in the query. |
|
Can you enable Github Actions for https://github.com/agam1092005/Nominatim/actions ? There's a section in https://github.com/osm-search/Nominatim/blob/master/CONTRIBUTING.md#workflow-for-pull-requests explaining how to do that. It will run the full test suite automatically for you. |
3 participants
Fixes #3750
Enable qualifier searches for unnamed POIs near small places.
Previously, queries like "Kingston pub" would fail to return unnamed amenities (pubs) located near a small place (Kingston village). The root cause was that qualifier+name search logic in SearchBuilder.build() only produced a
PlaceSearch with the qualifier as a filter, which doesn't work for unnamed POIs near small places.
Changes:
New QualifierNearSearch class (qualifier_search.py): A dedicated search type for qualifier+address queries. Unlike
NearSearch, it restricts inner results to actual places (rank_address < 30), only proceeds when there is a single matching place, and applies accuracy penalties for named POIs (+0.4) and multiple results (+0.2).
_build_qualifier_address_search() method in db_search_builder.py: When a query has qualifiers and a name but no near items, builds a QualifierNearSearch alongside the existing PlaceSearch. Temporarily modifies sdata in place (clearing qualifiers, isolating rankings) and restores it before returning. Only wraps PlaceSearch results, skipping AddressSearch.
No changes to NearSearch: All custom logic is contained in the new search type, leaving existing near searches (e.g., "pub in Kingston") unaffected.
Test Results:
make lint: passed
make mypy: passed (152 source files, no issues)
make bdd: 776 passed, 4 skipped
New BDD scenarios: qualifier search for unnamed pubs near Kingston, [amenity=restaurant] Vaduz API endpoint test
Tested on actual Nominatim installation using the --nominatim-purge flag
AI usage
AI tool (GitHub Copilot) was used for planning approaches and suggesting possible testing scenarios.
All implementation, code modifications, and final validation were manually reviewed and tested on the actual Nominatim installation using the --nominatim-purge flag.
Contributor guidelines (mandatory)