French regions no more in lookup

[Joxit]

Since the changes whosonfirst-data/whosonfirst-data#1213, the French regions are no longer in lookup.

WOF team set the mz:hierarchy_label to 0 for macroregion records in France which is filtered here : https://github.com/pelias/wof-admin-lookup/blob/d9abfe32ed40184bd657df463e2faeb6ff2f7326/src/pip/components/filterOutUnimportantRecords.js#L9-L10

This is a problem because in France we keep regions in the hierarchy even if they are not in the label.

Here is the whosonfirst properties for hierarchy_label_macroregion.

This is the list of all macroregion with hierarchy_label = 0

macroregion	country
Lapland	Finland
Northern Finland	Finland
Southwest Finland	Finland
West Finland	Finland
Southern Finland Province	Finland
Eastern Finland	Finland
Corsica	France
Pays de la Loire	France
Ile-of-France	France
Centre	France
Brittany	France
Provence-Alpes-Cote d'Azur	France
Normandy	France
Occitania	France
Upper France	France
Burgundy-Franche-Comte	France
Great East	France
New Aquitaine	France
Auvergne-Rhone-Alpes	France

An exception could be added for macroregions but I don't know if Finland needs macroregions in its hierarchies.

[orangejulius]

Yikes @Joxit, thanks for the report.

I'd prefer this is fixed in WOF if possible, but if not we would definitely support patching in wof-admin-lookup if necessary.

@stepps00/@nvkelso, is this a change you'd consider reverting?

[nvkelso]

It's expected that these would be different with the change.

The WOF property was added specifically for string label generation and it seems like @Joxit agrees that the macroregion shouldn't be included there. So huzzah, no WOF change to revert.

My opinion it's a Pelias change to separate the full (and true) hierarchy response from the label (address) construction. @orangejulius how is that setup now?

[orangejulius]

Oh I see. They were never in the label but now are not in the response bodies at all.

Yes I agree, it's a Pelias change then. Sorry for the noise WOF team.

@Joxit, now that mz:hierarchy_label is no longer a good single determinant of whether we should load a record for admin lookup, do you have suggestions for a change to that function?

[nvkelso]

@orangejulius are you not returning the full & true hierarchy from WOF in the lookup each time? I'd only expect the label (and if you wanted to an address message) to change.

[Joxit]

Yes @orangejulius macroregions are no more in the response body and the WOF team is doing this right, the property should be 0 for France.

I do not know what will be the side effects if we remove the filter filterOutUnimportantRecords. I think for now, we can only filter macroregions and see if we can remove filterOutUnimportantRecords forever (I will check which wof geojson will be affected).

[orangejulius]

Looking at https://github.com/pelias/wof-pip-service/pull/41 (the PR that originated filterOutUnimportantRecords), it appears the original intent was only to filter out neighbourhoods that are not "important" enough to bother including. So I think we can replace the logic in filterOutUnimportantRecords with something like this:

return wofData.properties['wof:placetype'] !== 'neighbourhood' || wofData.properties['mz:hierarchy_label'] === 1;

Basically, restricting the check against mz:hierarchy_label to only records in the neighbourhood layer. Let me know if that makes sense and works for you.

[nvkelso]

FWIW: we rest most the funky neighbourhood polygons to points so even that may be unnecessary now?

[Joxit]

Here are the stats with mz:hierarchy_label=0

county: 1
localadmin: 2
macrohood: 6
macroregion: 19
neighbourhood: 55208
region: 17

So Yes @orangejulius, this makes sense and should be good for me. I can do the PR tomorrow if you want it's 11PM here ^^'. @nvkelso You are right, most of neigbourhood are Point. But in these 55208 neighbourhood, I have 53135 Point and 2059 MultiPolygon/Polygon.

[orangejulius]

Great analysis. In that case let's just remove the entire filterOutUnimportantRecords stream. Less code is better! A PR would be great if you have the time.

[Joxit]

Fine :)

Sorry @orangejulius, but I can't push new branch to this repository :/

[missinglink]

@Joxit I just changed the repo settings, please try again.

[Joxit]

Yes, It works thanks :smile: