Skip to content

Conversation

@SebastienTainon
Copy link
Contributor

When using Nominatim provider, the API can return several kinds of responses for "city" for the same kind of query.
For example:

75004, France
https://nominatim.openstreetmap.org/search?q=75004,France&format=xml&addressdetails=1&limit=5

<suburb>Quartier Saint-Merri</suburb>
<city_district>4th Arrondissement</city_district>
<city>Paris</city>
<county>Paris</county>
<state>Ile-de-France</state>
<country>France</country>
<postcode>75004</postcode>

=> we receive the city in a "city" field

98000, Monaco
https://nominatim.openstreetmap.org/search?q=98000%2C+Monaco&format=xml&addressdetails=1&limit=5

<suburb>La Condamine</suburb>
<town>Monaco</town>
<postcode>98000</postcode>
<country>Monaco</country>

=> we receive the city field in a "town" field

Here is the wiki for Nominatim structure: http://wiki.openstreetmap.org/wiki/Nominatim

My proposition is that if the "city" field is absent from the response, we use the "town" field instead to define the city of the response.

@willdurand willdurand requested a review from Nyholm September 12, 2017 15:06
Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good. I like it.

@DaxServer
Copy link
Contributor

Just to extend the hierarchy, the locality can be returned as village and hamlet as well! Probably adding them as well might be good.

@SebastienTainon
Copy link
Contributor Author

@SrihariThalla Do you have an example of a Nominatim URL returning an hamlet? It does not appear in the documentation.
What about checking first city, then town, and then village if the two previous ones do not exist?

@DaxServer
Copy link
Contributor

Here are the links, @SebastienTainon :

The preferred order would be like you said:

  1. city
  2. town
  3. village
  4. hamlet

@SebastienTainon
Copy link
Contributor Author

@SrihariThalla I've updated the PR with tht modifications you are suggesting. Could you check that it's ok for you? 😄

Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you both.

Im just waiting for confirmation from @SrihariThalla and I'll be happy to merge this

@SebastienTainon SebastienTainon changed the title In Nominatim provider, use town if city is not defined In Nominatim provider, use city, then town, then village, then hamlet Sep 18, 2017
@Nyholm
Copy link
Member

Nyholm commented Nov 22, 2017

@SebastienTainon could you rebase on master so I can merge?

@SebastienTainon SebastienTainon force-pushed the master branch 3 times, most recently from fb30d5d to 8d89bb3 Compare November 22, 2017 09:15
@SebastienTainon
Copy link
Contributor Author

I've rebased and squashed @Nyholm :)

@Nyholm
Copy link
Member

Nyholm commented Nov 22, 2017

Can you also fix (the new) style CI stuff?

@SebastienTainon
Copy link
Contributor Author

It was related to #789 but no problem, I've fixed it too ;)

@DaxServer
Copy link
Contributor

Sorry, I lost track of this :-/ All Nominatim changes look good 👍

Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent. Thank you!

@Nyholm Nyholm merged commit 5e18088 into geocoder-php:master Nov 28, 2017
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