pelias / wof-admin-lookup

Who's on First Admin Lookup for the Pelias Geocoder
https://pelias.io
MIT License
9 stars 24 forks source link

Only change locality if borough is _not_ changed #297

Closed orangejulius closed 4 years ago

orangejulius commented 4 years ago

Because we now manage postal cities replacements for both borough and locality values, we have to be a bit careful about when we replace each of them.

After investigating what looked like an error in our logic for determining the best postal cities match in https://github.com/pelias/wof-admin-lookup/issues/288, it turns out a significant issue we were seeing in Brooklyn was actually a more subtle bug.

In short, it looks like if we replace the borough field with with a postal city value, we should not replace the locality value.

Closes #288

missinglink commented 4 years ago

Hmm yeah it's odd that I went sofar as to add a code comment with this exact situation in the code but didn't consider this ;)

orangejulius commented 4 years ago

Hmm yeah it's odd that I went sofar as to add a code comment with this exact situation in the code but didn't consider this ;)

Sometimes a problem guides you in a certain direction and there's usually a reason for that :)