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

consider lifecycle prefixes in osmTagSuggestingArea #8881

Merged
merged 2 commits into from
Jun 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions modules/osm/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ export {
osmOneWayTags,
osmPavedTags,
osmIsInterestingTag,
osmLifecyclePrefixes,
osmRemoveLifecyclePrefix,
osmRoutableHighwayTagValues,
osmFlowingWaterwayTagValues,
osmRailwayTrackTagValues
Expand Down
34 changes: 31 additions & 3 deletions modules/osm/tags.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,33 @@ export function osmIsInterestingTag(key) {
key.indexOf('tiger:') !== 0;
}

export const osmLifecyclePrefixes = {
// nonexistent, might be built
proposed: true, planned: true,
// under maintentance or between groundbreaking and opening
construction: true,
// existent but not functional
disused: true,
// dilapidated to nonexistent
abandoned: true, was: true,
tyrasd marked this conversation as resolved.
Show resolved Hide resolved
// nonexistent, still may appear in imagery
dismantled: true, razed: true, demolished: true, destroyed: true, removed: true, obliterated: true,
// existent occasionally, e.g. stormwater drainage basin
intermittent: true
};

/** @param {string} key */
export function osmRemoveLifecyclePrefix(key) {
const keySegments = key.split(':');
k-yle marked this conversation as resolved.
Show resolved Hide resolved
if (keySegments.length === 1) return key;

if (keySegments[0] in osmLifecyclePrefixes) {
return key.slice(keySegments[0].length + 1);
}

return key;
}

export var osmAreaKeys = {};
export function osmSetAreaKeys(value) {
osmAreaKeys = value;
Expand Down Expand Up @@ -44,13 +71,14 @@ export function osmTagSuggestingArea(tags) {
if (tags.area === 'no') return null;

var returnTags = {};
for (var key in tags) {
for (var realKey in tags) {
const key = osmRemoveLifecyclePrefix(realKey);
if (key in osmAreaKeys && !(tags[key] in osmAreaKeys[key])) {
returnTags[key] = tags[key];
returnTags[realKey] = tags[realKey];
return returnTags;
}
if (key in osmAreaKeysExceptions && tags[key] in osmAreaKeysExceptions[key]) {
returnTags[key] = tags[key];
returnTags[realKey] = tags[realKey];
return returnTags;
}
}
Expand Down
15 changes: 2 additions & 13 deletions modules/renderer/features.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { dispatch as d3_dispatch } from 'd3-dispatch';

import { prefs } from '../core/preferences';
import { osmEntity } from '../osm';
import { osmEntity, osmLifecyclePrefixes } from '../osm';
import { utilRebind } from '../util/rebind';
import { utilArrayGroupBy, utilArrayUnion, utilQsString, utilStringQs } from '../util';

Expand Down Expand Up @@ -42,17 +42,6 @@ export function rendererFeatures(context) {
'pedestrian': true
};

var past_futures = {
'proposed': true,
'construction': true,
'abandoned': true,
'dismantled': true,
'disused': true,
'razed': true,
'demolished': true,
'obliterated': true
};

var _cullFactor = 1;
var _cache = {};
var _rules = {};
Expand Down Expand Up @@ -210,7 +199,7 @@ export function rendererFeatures(context) {

for (var i = 0; i < strings.length; i++) {
var s = strings[i];
if (past_futures[s] || past_futures[tags[s]]) { return true; }
if (osmLifecyclePrefixes[s] || osmLifecyclePrefixes[tags[s]]) return true;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense that intermittent:* would be filtered out by the past/future filter? (Also not:*, discussed above.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure what the right answer is for intermittent:* is, but bear in mind that intermittent:* is only used once in the world (cf. other prefixes like disused:* are used >200,000 times). It seems like intermittent=yes is the more common tag.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the intermittent entry originated from 494e247. It was probably intended to support intermittent=yes, not intermittent:*=*.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for finding that. I guess it would be logical for the past/future filter to match all the lifecycle prefixes except intermittent

}
return false;
});
Expand Down
17 changes: 2 additions & 15 deletions modules/svg/tag_classes.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { select as d3_select } from 'd3-selection';
import { osmPathHighwayTagValues, osmPavedTags, osmSemipavedTags } from '../osm/tags';
import { osmPathHighwayTagValues, osmPavedTags, osmSemipavedTags, osmLifecyclePrefixes } from '../osm/tags';


export function svgTagClasses() {
Expand All @@ -9,20 +9,7 @@ export function svgTagClasses() {
'leisure', 'military', 'place', 'man_made', 'route', 'attraction',
'building:part', 'indoor'
];
var statuses = [
// nonexistent, might be built
'proposed', 'planned',
// under maintentance or between groundbreaking and opening
'construction',
// existent but not functional
'disused',
// dilapidated to nonexistent
'abandoned',
// nonexistent, still may appear in imagery
'dismantled', 'razed', 'demolished', 'obliterated',
// existent occasionally, e.g. stormwater drainage basin
'intermittent'
];
var statuses = Object.keys(osmLifecyclePrefixes);
var secondaries = [
'oneway', 'bridge', 'tunnel', 'embankment', 'cutting', 'barrier',
'surface', 'tracktype', 'footway', 'crossing', 'service', 'sport',
Expand Down
23 changes: 23 additions & 0 deletions test/spec/osm/tags.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
describe('iD.osmRemoveLifecyclePrefix', function () {
it('removes the lifecycle prefix from a tag key', function () {
expect(iD.osmRemoveLifecyclePrefix('was:natural')).to.equal('natural');
expect(iD.osmRemoveLifecyclePrefix('destroyed:seamark:type')).to.equal('seamark:type');
});

it('ignores invalid lifecycle prefixes', function () {
expect(iD.osmRemoveLifecyclePrefix('ex:leisure')).to.equal('ex:leisure');
});
});


describe('osmTagSuggestingArea', function () {
beforeEach(function () {
iD.osmSetAreaKeys({ leisure: {} });
});

it('handles features with a lifecycle prefixes', function () {
expect(iD.osmTagSuggestingArea({ leisure: 'stadium' })).to.eql({ leisure: 'stadium' });
expect(iD.osmTagSuggestingArea({ 'disused:leisure': 'stadium' })).to.eql({ 'disused:leisure': 'stadium' });
expect(iD.osmTagSuggestingArea({ 'ex:leisure': 'stadium' })).to.be.null;
});
});