pelias / openstreetmap

Import pipeline for OSM in to Pelias
MIT License
112 stars 72 forks source link

chore: upgrade pelias-model to latest version #554

Closed missinglink closed 3 years ago

missinglink commented 3 years ago

I was investigating why the autocomplete query "Statue of Liberty" was not returning https://www.openstreetmap.org/way/32965412 when I discovered that the pelias/model version was well out-of-date and missing some features from the last year or so.

We were pinning ^7.2.1 which is from 10 June 2020, this PR brings it up-to-date with the latest ^9.1.0

Screenshot 2021-07-20 at 13 13 10

https://github.com/pelias/model/tags

In particular 8.1.0 is very beneficial because it will reduce some of the TF/IDF scoring issues such as for the Statue of Liberty.

missinglink commented 3 years ago

I'm super confused about the ru change to the fixture in this PR. [edit] agh there was a bug introduced in 8.1.0, I'll open a PR to fix it.

missinglink commented 3 years ago

I've fixed the issue surfaced by the end-to-end tests and changed the commit to point to the new latest version:

Screenshot 2021-07-20 at 14 43 51
missinglink commented 3 years ago

Since rebasing this we unfortunately lost the original diff which showed the ru issue, it looked like this:

diff --git a/test/fixtures/combined_vancouver_queens.json b/test/fixtures/combined_vancouver_queens.json
index d653556..a53375a 100644
--- a/test/fixtures/combined_vancouver_queens.json
+++ b/test/fixtures/combined_vancouver_queens.json
@@ -930940,15 +930940,13 @@
         "default": [
           "BC Place",
           "B.C. Place"
-        ],
-        "ru": "Би-Си Плэйс"
+        ]
       },
       "phrase": {
         "default": [
           "BC Place",
           "B.C. Place"
-        ],
-        "ru": "Би-Си Плэйс"
+        ]
       },
       "center_point": {
         "lon": -123.112021,
orangejulius commented 3 years ago

Oh man, this is a good catch. There have definitely been some important changes in pelias-model so it will be good to get them in.

I think originally most of the changes were prompted by us noticing issues in the whosonfirst importer, but clearly they are applicable elsewhere. Maybe we should go through and check to make sure all the other modules are up to date everywhere.