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

City and State Lookup #31

Closed
wants to merge 1 commit into from
Closed

City and State Lookup #31

wants to merge 1 commit into from

Conversation

djskripta
Copy link

Partially related to #20
By no means a complete implementation, and lacks docs and unit tests.
I figured i would post this as a starting point since i've already coded these features for my own project.

Partially related to symerio#20 
By no means a complete implementation, and lacks docs and unit tests.
I figured i would post this as a starting point since i've already coded these features for my own project.
@rth
Copy link
Member

rth commented Mar 20, 2020

Thanks @djskripta! We would indeed need to add associated tests to test_pgeocode.py.

One concern I have is that creating additional classes for each query will make the API a bit less readable. Can we not add this directly as Nominatim.query_city and ``Nominatim.query_state` methods. Or is the issue that we need to pre-compute corresponding aggregations?

We could rename _index_postal_codes to _create_index(self, kind='postal_code') and make it work for states and cities. That would would allow to re-use code as well. The fact that we would need to create all these index files when initializing Nominatim might not be too much of an issue I think.

@rth
Copy link
Member

rth commented Mar 20, 2020

Actually I'm not sure we really need to create indexes for states and cities. That was done because postal codes were expected to be unique. For cities and particularly states, there would be multiple postal codes per query. So maybe just something along the lines,

mask = df['county_code'].str.contains(query)
return df[mask]

(with some additional string & query normalization) would be enough? It shouldn't be too bad performance wise.

@azmeuk
Copy link
Contributor

azmeuk commented Apr 7, 2021

@djskripta Do you have any intentions to solve those suggestions? May I help you with anything?

@rth
Copy link
Member

rth commented Apr 7, 2021

@azmeuk Feel free to continue this work in a separate PR. Thanks!

@rth
Copy link
Member

rth commented Dec 13, 2022

Thanks for the work on this PR! This is now implemented with #59:

>>> import pgeocode
>>> nomi = pgeocode.Nominatim('fr')
>>> nomi.query_location('Paris', col='place_name')
   country_code     postal_code place_name  ... latitude  longitude accuracy
0            FR           75000      Paris  ...  

[100 rows x 12 columns]

>>> nomi.query_location('Île-de-France', col='state_name')
...

Closing as resolved.

@rth rth closed this Dec 13, 2022
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