mapbox / mapbox-sdk-rb

A Ruby interface to Mapbox APIs.
Other
59 stars 32 forks source link

Permanent & batch geocoding #40

Closed waissbluth closed 5 years ago

waissbluth commented 5 years ago

Does/will this package support calling the permanent geocoding endpoint and batch geocoding?

samfader commented 5 years ago

@waissbluth Not currently. Looking quickly:

Adding a permanent endpoint would require adding places.permanent as an option here (https://github.com/mapbox/mapbox-sdk-rb/blob/master/lib/mapbox/geocoder.rb#L37) and here (https://github.com/mapbox/mapbox-sdk-rb/blob/master/lib/mapbox/geocoder.rb#L46) and possibly some other changes.

Adding the ability to batch geocode would first require the permanent option being added, and then probably a new method (i.e. geocode_batch) under this method https://github.com/mapbox/mapbox-sdk-rb/blob/master/lib/mapbox/geocoder.rb#L46.

If you feel comfortable working on this, please feel free to open a PR and I'd be happy to take a look. In the meantime, I'll add this to the backlog.

waissbluth commented 5 years ago

Hi @samfader, it looks like calling simply Mapbox::Geocoder.geocode_forward("1 main street", {}, 'mapbox.places-permanent') does work and calls the permanent endpoint. Are there any other changes you think would be required? Is there something that I am missing?

samfader commented 5 years ago

@waissbluth It appears I read the code incorrectly 😅 You are totally right, the default option is mapbox.places but you can pass in mapbox.places-permanent and override the default.

You can also batch geocode with the current functionality:

places = Mapbox::Geocoder.geocode_forward("{Chester, NJ};{Philadelphia, PA}", {:proximity => {:longitude => -74.6968, :latitude => 40.7843}}, 'mapbox.places-permanent')

See syntax in docs here: https://docs.mapbox.com/api/search/#batch-geocoding

Let me know if this works for you.

waissbluth commented 5 years ago

@samfader yes it does -- thanks! It still would be nice to able to pass it an array rather than a joined string (especially considering they need to be URL-encoded), but this will do.

samfader commented 5 years ago

@waissbluth No problem. If you have spare time and want to work on that addition, happy to review/merge it in!