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

Configure countries to use for postal city lookup #305

Open
NairolfL opened this issue May 5, 2021 · 4 comments
Open

Configure countries to use for postal city lookup #305

NairolfL opened this issue May 5, 2021 · 4 comments

Comments

@NairolfL
Copy link

NairolfL commented May 5, 2021

Hello,
I've been trying this wof admin lookup with different pelias importers (mostly openstreetmap / openaddresses).
I may be missing something but it seems impossible to configure which country to use for the postal city lookup.
It seems that the country list is hard coded at : https://github.com/pelias/wof-admin-lookup/blob/master/src/postalCityMap.js#L53

Would it be possible to make this configurable ? Or is it already and I missed it ?
I see that data exists for a lot of countries already.

@orangejulius
Copy link
Member

Hi @NairolfL,
You're right, it's probably harder to edit that list than it needs to be. Especially because it's buried in a dependency of the importers, rather than the importers themselves.

We use the pelias/config module to allow changing all sorts of options within Pelias, and this would be a good candidate for that sort of thing.

@NairolfL
Copy link
Author

NairolfL commented May 6, 2021

Hi @orangejulius

Thanks for you quick response.
Would you accept a merge request if I take some time to update it ?

@orangejulius
Copy link
Member

Absolutely, that would be great!

I think if you name the config option imports.adminLookup.postalCitiesCountries and allow it to be an array of any length, that would be nice. It would line up well with some of our existing config options like the directory configuration (which is not documented).

You can keep the defaults currently in the file if the parameter is empty, otherwise it should load files if it exists. I think we can limit it to only 3-character country codes to keep things simpler.

If you have any other questions let us know, or feel free to open an early draft PR for review.

@NairolfL
Copy link
Author

Hello @orangejulius,
I did a first draft of the change.
I tested it with the openaddresses importer and it worked.
It defaults to the previous values so it shouldn't break anything.
I have a few questions :

  • Should I add some tests for it ?
  • Should I change the default value in pelias-config ?
  • Should I add a check to make sure we get a strings array ? I didn't see other validations like this in the code so I didn't.

Thank you !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants