Add Gisgraphy Geocoder

[gisgraphy]

Add Gisgraphy Geocoder as a new provider (for geocoding forward and reverse and autocomplete) Documentation need to be updated. Some technical choice can be discussed as

• the CountryInfo class
• what to do if the queryType is not correct (we then consider that it is geocoding), maybe throw an exception can be better
• put the DTO in a specific package can improve lisibility.

A try in real environement can also be necessary

Every comments are welcomed

[karussell]

Thanks a lot! Will have a look, especially in the choices you mean.

[boldtrn]

what to do if the queryType is not correct (we then consider that it is geocoding), maybe throw an exception can be better

I think it might be better to have it similar to the GraphHopper api with reverse=true|false and maybe add another parameter with autocomplete=true|false. That way users don't necessarily rewrite the request when using provider=gisgraphy.

[gisgraphy]

If it is ambiguous for us, it will be ambiguous for the users, I suggest to notify user by an exception that says that something is not logic in the query and propose to set reverse or autocomplete to false; it is not our role to choose.

Le 15/02/2018 à 13:07, Peter a écrit :

@karussell commented on this pull request.

---

In src/main/java/com/graphhopper/converter/resources/ConverterResourceGisgraphy.java https://github.com/graphhopper/geocoder-converter/pull/34#discussion_r168456640:

+

• public ConverterResourceGisgraphy(String geocodingUrl,
• String reverseGeocodingUrl, String searchURL, String apiKey,
• Client jerseyClient) {
• this.geocodingUrl = geocodingUrl;
• this.reverseGeocodingUrl = reverseGeocodingUrl;
• this.searchURL = searchURL;
• this.jerseyClient = jerseyClient;
• this.apiKey = apiKey;
• }
• @GET
• @Timed
• public Response handle(@QueryParam("q") @DefaultValue("") String query,
• @QueryParam("lat") @DefaultValue("") String lat,
• @QueryParam("lng") @DefaultValue("") String lng,

Good question. Maybe if autocomplete==true and reverse==true then we could do an autocomplete with location bias :) ?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/graphhopper/geocoder-converter/pull/34#discussion_r168456640, or mute the thread https://github.com/notifications/unsubscribe-auth/AHS1SGrmWUrmiYyMwN8No6DX-uEwqY8_ks5tVB3kgaJpZM4SFF3o.

[karussell]

ok, true.

[gisgraphy]

modifications done ! let me know if it is ok to be merged

[devemux86]

Also could use StringBuilder instead of StringBuffer where possible?

[gisgraphy]

I have done changes to use string builder. Some changes has also been done to display zipcode in the label.

I remove WIP from the name of the PR ;-)

[gisgraphy]

There is still some docs and README to update

[gisgraphy]

README updated for Gisgraphy.

Everything ok to be merged ?

[gisgraphy]

modifications done. Tests are green

[gisgraphy]

Is there something missing to merge ?

[boldtrn]

BTW: I have just seen that you used tabs instead of spaces. The whole codebase uses spaces, which will lead to ugly changelogs with the next autoformat. Most editors nowadays use spaces by default and also most language specs recommend using spaces. If it's easy for to changes this, it would be nice if you could do this, otherwise I can do this in another commit before merging.

[boldtrn]

Is there something missing to merge ?

Sorry for the multiple review rounds, I know it's tedious :). I added a couple more comments.

[gisgraphy]

no problem, I appreciate review since it makes sense :) hope it will be ok.

[boldtrn]

Thanks, the formatting looks correct now, you can always check in the files tab. Before there were huge changes in files where you only changed one or two lines.

I think once we fix the failing test, this PR should be pretty much ready to merge. We just need to decide how we handle the country code :).

[gisgraphy]

modifications are done. For the country field, I plan to do server modifications soon and will propose a new PR when done

do we merge? :)

[karussell]

Thank you all will now bring this into production - exciting :) !

[karussell]

@gisgraphy is it already possible to search for parking places through this converter?

[gisgraphy]

to search by name, the URL is something like http://services.gisgraphy.com/fulltext/?q=sebastopol&style=long&placetype=parking

or name / GPS : http://services.gisgraphy.com/fulltext/?q=sebastopol&style=long&placetype=parking&lat=48.8566101&lng=2.3514992

fulltext webservice is possible but the placetype parameter is not mapped in the converter

or by GPS position : http://services.gisgraphy.com/geoloc/search?format=json&lat=48.856610100000005&lng=2.3514992&placetype=Parking

geoloc webservice is not available yet in the converter.

so it need some developpement

[gisgraphy]

there is a lot of possibilities in Gisgraphy that are possible but this PR was a first one. maybe add an enhancement/issues to track this. I am currently busy and have no time to do that soon :(

[karussell]

Really no problem and no hurry. Will create an issue and maybe @boldtrn or I will create time for this

46