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

Adding address unit attribute to queries #77

Merged
merged 4 commits into from
Nov 20, 2017

Conversation

sweco-semhul
Copy link
Contributor

To resolve pelias/pelias#618 more details found in comments to that issue

@dianashk
Copy link
Contributor

dianashk commented Nov 2, 2017

Hi @sweco-semhul! 👋 Thank you so much for this awesome PR (and the other corresponding ones). We are really excited about this feature and have talked about prioritizing very recently so this is great timing!

We'll have a look at the code very soon and get back with feedback, but at first glance, it looks great.

In order for this functionality to actually work in production we'll need to update our openaddresses importer to stop throwing away unit information. Currently we do not store unit numbers in our Elasticsearch schema so these new queries wouldn't be able to find much just yet.

@sweco-semhul
Copy link
Contributor Author

@dianashk thanks for your kind words, happy to help!

As you have probably already seen, mentioned in the main issue, there are corresponding changes for that in schema pelias/schema#258 and the openaddresses importer pelias/openaddresses#320

Yes data needs to be reloaded and there were a lot of parts that needed to be touched to make this change and it is/was a bit tricky to test the full change.

Let me know if I can be of any help getting this further.

Copy link
Member

@orangejulius orangejulius left a comment

Choose a reason for hiding this comment

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

This looks good and worked for me with Portland addresses :)

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

Successfully merging this pull request may close these issues.

Add unit numbers to addresses
3 participants