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

Updated some german abbreviations & added support for fractional housenumbers #159

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

arnesetzer
Copy link

@arnesetzer arnesetzer commented Jun 21, 2022

👋 I did some awesome work for the Pelias project and would love for everyone to have a look at it and provide feedback.


Here's the reason for this change 🚀

  • Got a project with these type of housenumbers
  • Other project, where "bei Berlin" (near Berlin) won't be found caused by the name "b.Berlin"

Here's what actually got changed 👏


Here's how others can test the changes 👀

Import the german osm data and try to find these buildings by their address. As far as I concerned these are written down correctly in OSM

@missinglink
Copy link
Member

hi @arnesetzer there is an issue running CI for external contributors.
I hopefully have it fixed in #160

Please rebase master against your branch and push to trigger the CI

@arnesetzer arnesetzer requested a review from Joxit August 17, 2022 13:23
@missinglink
Copy link
Member

@arnesetzer the unit tests are failing, you can run them locally with the command npm test

@@ -15,7 +15,8 @@ class HouseNumberClassifier extends WordClassifier {
/^(\d{1,5})[a-zA-Z\u0400-\u04FF]?\/(\d{1,5})$/.test(span.body) || // 1/135 or 1b/135 Style
/^(\d{1,5})([nsewNSEW])(\d{1,5})[a-zA-Z]?$/.test(span.body) || // 6N23 Style (ie Kane County, IL)
/^([nsewNSEW])(\d{1,5})([nsewNSEW]\d{1,5})?$/.test(span.body) // W350N5337 or N453 Style (ie Waukesha County, WI)
Copy link
Member

Choose a reason for hiding this comment

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

Missing || at the end of the line here

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