From ee9d9ae0be61a242eca0cd0b1aac095f1980eca4 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Fri, 5 Aug 2016 10:55:14 -0400 Subject: [PATCH 1/2] Use lbl:bbox property when present After the discussion in https://github.com/pelias/pelias/issues/370, Who's on First implemented the `lbl:bbox` field, which is a hand-made bounding box for areas where the mathematically derived bounding box doesn't match up with what people generally want. At least two of these were implemented in https://github.com/whosonfirst-data/whosonfirst-data/issues/361, so it's time to start using them! Connects https://github.com/pelias/pelias/issues/370 Connects https://github.com/pelias/pelias/issues/397 --- src/components/extractFields.js | 10 ++++++- test/components/extractFieldsTest.js | 39 ++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/components/extractFields.js b/src/components/extractFields.js index 7200ac14..1b19dc23 100644 --- a/src/components/extractFields.js +++ b/src/components/extractFields.js @@ -38,6 +38,14 @@ function getLon(properties) { } } +function getBoundingBox(properties) { + if (properties['lbl:bbox']) { + return properties['lbl:bbox']; + } else { + return properties['geom:bbox']; + } +} + /* This function extracts the fields from the json_object that we're interested in for creating Pelias Document objects. If there is no hierarchy then a @@ -54,7 +62,7 @@ module.exports.create = function map_fields_stream() { parent_id: json_object.properties['wof:parent_id'], lat: getLat(json_object.properties), lon: getLon(json_object.properties), - bounding_box: json_object.properties['geom:bbox'], + bounding_box: getBoundingBox(json_object.properties), iso2: json_object.properties['iso:country'], population: getPopulation(json_object.properties), popularity: json_object.properties['misc:photo_sum'] diff --git a/test/components/extractFieldsTest.js b/test/components/extractFieldsTest.js index a42074d3..2303c41d 100644 --- a/test/components/extractFieldsTest.js +++ b/test/components/extractFieldsTest.js @@ -552,4 +552,43 @@ tape('readStreamComponents', function(test) { t.end(); }); }); + + test.test('label bounding box should take precedence over math bounding box', function(t) { + var input = [ + { + id: 12345, + properties: { + 'wof:name': 'wof:name value', + 'wof:placetype': 'county', + 'wof:parent_id': 'parent id', + 'geom:latitude': 12.121212, + 'geom:longitude': 21.212121, + 'geom:bbox': '-13.691314,49.909613,1.771169,60.847886', + 'lbl:bbox': '-14.691314,50.909613,2.771169,61.847886', + 'qs:a2_alt': 'qs:a2_alt value' + } + } + ]; + + var expected = [ + { + id: 12345, + name: 'wof:name value', + place_type: 'county', + parent_id: 'parent id', + lat: 12.121212, + lon: 21.212121, + iso2: undefined, + population: undefined, + popularity: undefined, + bounding_box: '-14.691314,50.909613,2.771169,61.847886', + abbreviation: undefined + } + ]; + + test_stream(input, extractFields.create(), function(err, actual) { + t.deepEqual(actual, expected, 'label geometry is used'); + t.end(); + }); + }); }); From cdf2372f922ba787978237dbd8feb5456c1023f2 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Tue, 9 Aug 2016 10:38:08 -0400 Subject: [PATCH 2/2] Check for existence of lbl:bbox property This is clearer than checking for truthiness --- src/components/extractFields.js | 2 +- test/components/extractFieldsTest.js | 39 ++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/components/extractFields.js b/src/components/extractFields.js index 1b19dc23..399087ed 100644 --- a/src/components/extractFields.js +++ b/src/components/extractFields.js @@ -39,7 +39,7 @@ function getLon(properties) { } function getBoundingBox(properties) { - if (properties['lbl:bbox']) { + if (properties.hasOwnProperty('lbl:bbox')) { return properties['lbl:bbox']; } else { return properties['geom:bbox']; diff --git a/test/components/extractFieldsTest.js b/test/components/extractFieldsTest.js index 2303c41d..7a9dc293 100644 --- a/test/components/extractFieldsTest.js +++ b/test/components/extractFieldsTest.js @@ -591,4 +591,43 @@ tape('readStreamComponents', function(test) { t.end(); }); }); + + test.test('label bounding box should take precedence over math bounding box even if empty', function(t) { + var input = [ + { + id: 12345, + properties: { + 'wof:name': 'wof:name value', + 'wof:placetype': 'county', + 'wof:parent_id': 'parent id', + 'geom:latitude': 12.121212, + 'geom:longitude': 21.212121, + 'geom:bbox': '-13.691314,49.909613,1.771169,60.847886', + 'lbl:bbox': '', + 'qs:a2_alt': 'qs:a2_alt value' + } + } + ]; + + var expected = [ + { + id: 12345, + name: 'wof:name value', + place_type: 'county', + parent_id: 'parent id', + lat: 12.121212, + lon: 21.212121, + iso2: undefined, + population: undefined, + popularity: undefined, + bounding_box: '', + abbreviation: undefined + } + ]; + + test_stream(input, extractFields.create(), function(err, actual) { + t.deepEqual(actual, expected, 'label geometry is used'); + t.end(); + }); + }); });