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

feat(poland): classifying more addresses for poland #174

Merged
merged 16 commits into from
Aug 15, 2023

Conversation

JanF01
Copy link
Contributor

@JanF01 JanF01 commented Jul 24, 2023

👋 Adding Poland to the list of countries using a StreetPrefix as well as activating the PostcodeClassifier and PlaceClassifier for Poland. Adding support for a new Street Scheme


Here's the reason for this change 🚀

We are using the autocomplete query and we have found that many of the Polish streets were not recognized as such, we have also found problems with multi-word street names, but if Polish street prefixes will be recognized, than we could create a workaround.

We have also found that the Polish Postcodes are not recognized either. It would be great if we could get those to work 👍

Here's how others can test the changes 👀

I have written tests to see if things are working correctly

@missinglink
Copy link
Member

Hi @JanF01 the unit test suite is currently failing, could you please have a look?

@JanF01
Copy link
Contributor Author

JanF01 commented Jul 25, 2023

Things should be running now. Turned out that one of the tests was an edge case, which has been specified before by @mansoor-sajjad here:

@JanF01
Copy link
Contributor Author

JanF01 commented Jul 25, 2023

Additionaly we have found a problem with street names that have this scheme: Prefix Place (Name/Adjective) or
Prefix Numeric (Name/Adjective).

I'm not shure if it's too invasive, but I have added support for those scheme in the file:

https://github.com/pelias/parser/tree/master/classifier/scheme/street.js

I have checked locally if all the tests are going through.

@missinglink
Copy link
Member

missinglink commented Jul 26, 2023

I'm not shure if it's too invasive, but I have added support for those scheme in the file.

Generally speaking it's ok to make code changes as long as they don't break existing test cases.

Looks like these don't, which is great 👍

@Joxit
Copy link
Member

Joxit commented Jul 26, 2023

Hi @JanF01 and thank you for your contribution.

Could you move your change resources/libpostal/dictionaries/pl/synonyms.txt and resources/libpostal/dictionaries/pl/place_names.txt to the folder resources/pelias/dictionaries/libpostal/pl ? (create the new folder + files and add only new lines)

Resources in resources/libpostal/ are an extract from libpostal and may be overridden one day.

Copy link
Member

@Joxit Joxit left a comment

Choose a reason for hiding this comment

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

I pushed one commit for code formatting and diff cleaning

LGTM

@Joxit Joxit merged commit 7d28b15 into pelias:master Aug 15, 2023
3 checks passed
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