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

Integrate configurable filters for Pelias Lookup #1357

Closed

Conversation

jsantos
Copy link
Contributor

@jsantos jsantos commented Oct 19, 2018

Pelias supports filtering of results to a specific country, source or type of location (venue, region, etc).

Since there was already a custom configuration to set the pelias endpoint, I think it might be a good idea to customize the filtering parameters that one might need:

  • Only get venues as results;
  • Limit results to a specific country;
  • Limit results to the OSM dataset;

More integrations can be done, but I think at least these are a nice-to-have.

Some info on the API itself: https://github.com/pelias/documentation#core-features-and-api-documentation

  • Enable specification of custom filters on the initializer and apply these to the request params;
  • Added test for custom filters;
  • Updated the documentation on how to configure the Pelias lookup (these filters are optional);
  • Adding extra properties to Pelias Results:
    • place_name
    • street
    • source

Pelias offers some nice features on its API, such as filtering results
to a specific country, source of type of location (venue, region, etc).

Since there was already a custom configuration to set the pelias
endpoint, I think it might be a good idea to customize the filtering
parameters that one might need:

- Only get venues as results;
- Limit results to a specific country;
- Limit results to the OSM dataset;

More integrations can be done, but I think at least these are a nice-to-have.

Some info on the API itself: https://github.com/pelias/documentation#core-features-and-api-documentation

I've also updated the documentation on how to configure the Pelias
lookup (these filters are optional).
- place_name
- street
- source
@orangejulius
Copy link
Contributor

Hey @jsantos.
Pelias maintainer here. This looks good. It should help anyone using Pelias use more of its features and abilities :)

@jsantos
Copy link
Contributor Author

jsantos commented Nov 20, 2018

@orangejulius Any thing that needs correction here? I see one of the checks is failing, but seems to be a Travis issue:

Worker information
hostname: af5f4a90-2f66-4016-a98c-6bba0d3c878e@1.production-2-worker-org-05-packet
version: v4.0.0 https://github.com/travis-ci/worker/tree/e5cb567e10c0eefe380e81c9a2229ac8fb6a16ce
instance: 23c86e1 travisci/ci-garnet:packer-1512502276-986baf0 (via amqp)
startup: 2.325769863s
OCI runtime exec failed: exec failed: container_linux.go:348: starting container process caused "process_linux.go:86: executing setns process caused \"exit status 21\"": unknown

@alexreisner
Copy link
Owner

Thanks for your work on this. My concern is that these extra parameters can already be passed to Pelias by using the :params option to Geocoder.search. This PR just changes the syntax. And since Pelias actually supports a lot more parameters than the ones added by this PR, I think it's potentially confusing.

Generally speaking, every API supports a lot of optional parameters. The more of them Geocoder supports, the more work we have to do keeping them up to date (and with 40 APIs supported, this is not practical). Does that make sense?

@jsantos
Copy link
Contributor Author

jsantos commented Nov 20, 2018

@alexreisner it does, I can imagine how hard it is to keep all the API's up to date.

I bumped into this use case because one of the projects I'm working on was getting a lot of faulty results while reverse geocoding (e.g. an exact POI instead of an address). So my actual usage of this PR was aiming more to Geocoder.reverse_geocode. Haven't tried to use params: on it, but will give it a shot and document it if needed.

This is quite an edge case, and I can work around it by properly filtering the datasets I load into Pelias. It certainly requires less work than updating this every time an API changes :)

Thanks for the feedback, I love working with this gem and always willing to contribute in a better way!

EDIT: Ookay, actually I could use it as described in here: https://github.com/alexreisner/geocoder#advanced-model-configuration

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.

3 participants