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

Fixes undefined Property errors #4

Merged
merged 2 commits into from
Jan 7, 2019
Merged

Conversation

MathieuMaas
Copy link
Contributor

@MathieuMaas MathieuMaas commented Jan 2, 2019

Fixes the error: Undefined property: stdClass::$gemeentenaam and Undefined property: stdClass::$provincienaam

Fixes the error: Undefined property: stdClass::$gemeentenaam and Undefined property: stdClass::$provincienaam
@barryvdh
Copy link

barryvdh commented Jan 2, 2019

This seems to happen when we get a postcode type, instead of adres. Should perhaps the adres have a higher priority then the postcode?

@bbrala
Copy link
Member

bbrala commented Jan 4, 2019

Thanks for the PR! Being a bit defensive on the properties seems like a good idea. We prefer you use the PR template though so we know what is covered in a glance.

@barryvdh The perfect priorities are hard, but still it shouldn't really break. Guess we should extend the tests to at least also test the different result types.

@bbrala bbrala requested a review from JaZo January 4, 2019 13:45
@JaZo
Copy link
Member

JaZo commented Jan 7, 2019

@MathieuMaas, thanks for the PR, looks good to me!

@barryvdh, the priorities are indeed hard to set right for every situation. I have tried many values and the current values seem to give the best results in my tests. I have even tried to set the priority of an address to 10 and it still gives a postalcode as first result while the second is an address... The development team of PDOK knows the matching and sorting is not optimal and is working on an improved (v4) version.

@JaZo JaZo merged commit dffec64 into swisnl:master Jan 7, 2019
@JaZo
Copy link
Member

JaZo commented Jan 8, 2019

N.B. The missing data is caused by the recent municipal reclassifications: https://geoforum.nl/t/bij-bepaalde-adressen-wordt-er-geen-woonplaats-informatie-meer-meegestuurd/2283

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.

None yet

4 participants