Skip to content
This repository has been archived by the owner on Dec 4, 2018. It is now read-only.

add autocompletion support to OSM Scout geocoder #47

Merged
merged 3 commits into from
Jul 29, 2018

Conversation

rinigus
Copy link
Contributor

@rinigus rinigus commented Jul 19, 2018

This seems to be working. Several issues/questions:

  • expected: when you type partial name of the city/country, libpostal may not recognize it and place it under category house (so, way off in hierarchy). To assist, users would have to enable primitive parser and add , in the search string. Or use this feature mainly to quickly find some restaurant by its name.

  • unexpected: Looks like filterCompletions throws out results which may be valid, but not in its understanding. For example, searching for "Tallink Stockholm" would return "Tallink-Silja Line, Östermalms stadsdelsområde, Stockholm, Stockholm County, Svealand, 115 41, Sverige" with the title and label "Tallink-Silja Line, Östermalms stadsdelsområde". I wonder if there should be a property that would force to accept the completion if its coming from the geocoder? Notice that since we could search for Sweden and get Sverige in response. So, I would have trusted geocoder more than the text based filtering.

Question: What is the difference between title and label? In my case, I just put them the same.

I presume we need to resolve the issues before merging...

@otsaloma
Copy link
Owner

  • expected: If I understood correctly this is why Pelias has a separate endpoint for autocomplete, which doesn't use libpostal.

  • unexpected: Indeed, autocompletions shouldn't actually be futher filtered in QML, I'll fix that.

title/label: I don't know if this is really needed, but the background is that the previous result pages have a two-line display of title and description, with title relayed to the POI bubble. Autocompletions have a one-line display. Technically perhaps label should be a fully qualified name, e.g. "Address, City" while title could be just "Address" as in the POI bubble you no longer need a defining hierarchy.

@rinigus
Copy link
Contributor Author

rinigus commented Jul 19, 2018

title/label: OK, no problem, I can merge then full address into the label. Then we have

title: for POI display
description: what it is (Building, for example) and full admin region
label: fully qualified name (admin region in OSM Scout Server).

If this is OK, I'll make the change and update PR

Re Pelias: probably. I think I will still keep libpostal parsing and, one can always hope, it may educate users about parsing of their queries. But even with this flaw, I think the autocomplete is a very useful feature.

PS: Come winter, and we all will enjoy slightly warmer phones while typing our search queries!

@otsaloma
Copy link
Owner

That title/description/label looks fine.

@rinigus
Copy link
Contributor Author

rinigus commented Jul 19, 2018

The label has been adjusted accordingly.

Just stumbled on an issue: when entering some bogus string, autocomplete keeps trying to search for it when the returned results are empty []. I presume this is for all geocoders, not just OSM Scout Server. Although, don't want to spam other ones.

@otsaloma
Copy link
Owner

Autocomplete currently runs on a timer, once per second, unless there's an existing call in progress. Maybe I could limit that when the query hasn't changed, but providers should do their own caching anyway.

@rinigus
Copy link
Contributor Author

rinigus commented Jul 19, 2018

That's a tricky one. Results are cached, but only if we get hits (I think the same goes for all providers, most of the core code is the same among providers):

    if results and results[0]:
        cache[url] = copy.deepcopy(results)

Which, for the search, is a good default. For example, network was done (for online providers) or maps were missing (osm scout) or something else. So, after resolving such external issue, user can try again by pressing search.

For autocomplete though, we start spamming the search provider if the search string gets zero responses. So, to avoid it, maybe we should call autocomplete only if the query has changed when compared to the previous try. Then users can still hit the search if the full query was entered.

@otsaloma
Copy link
Owner

So, to avoid it, maybe we should call autocomplete only if the query has changed when compared to the previous try.

Yes, I'll make this change. But, still, I think you should fix your caching. It's very common on mobile to do typos and hitting backspace should not trigger a network request either. You could just simply save all autocomplete results

key = "autocomplete:{}".format(query)
with poor.util.silent(KeyError):
    return copy.deepcopy(cache[key])
results = geocode(query, params)
cache[key] = copy.deepcopy(results)
return results

@rinigus
Copy link
Contributor Author

rinigus commented Jul 19, 2018

Thanks for quick copy-and-paste guide! I have added it to the geocoder

@otsaloma otsaloma merged commit bd6d8c1 into otsaloma:master Jul 29, 2018
@rinigus rinigus deleted the osmscout_autogeo branch July 30, 2018 20:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants