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

Add denominations tree #5960

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
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
4 changes: 3 additions & 1 deletion app/src/Category.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,9 @@ export default function Category(props) {
let wikidataTag;
if (t === 'brands') {
wikidataTag = 'brand:wikidata';
} else if (t === 'flags') {
} else if (t === 'denominations') {
wikidataTag = 'denomination:wikidata';
}else if (t === 'flags') {
wikidataTag = 'flag:wikidata';
} else if (t === 'operators') {
wikidataTag = 'operator:wikidata';
Expand Down
5 changes: 5 additions & 0 deletions app/src/CategoryInstructions.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ export default function CategoryInstructions(props) {
itemType = 'brand';
logo = 'logos';
wikidataTag = 'brand:wikidata';
} else if (t === 'denominations') {
a = 'a';
itemType = 'denomination';
logo = 'a commons logo';
wikidataTag = 'denomination:wikidata';
} else if (t === 'flags') {
a = 'a';
itemType = 'flag';
Expand Down
11 changes: 11 additions & 0 deletions app/src/CategoryRow.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,17 @@ relation[${k}=${v}][brand=${bn}][brand:wikidata=${qid}]
{ color:green; fill-color:green; }
}}`;

} else if (t === 'denominations') {
n = item.tags.denomination;
kvn = `${k}/${v}|${n}`;
tags = item.tags || {};
qid = tags['denomination:wikidata'];
overpassQuery = `[out:json][timeout:100];
(nwr["denomination"="${n}"];);
out body;
>;
out skel qt;`;

} else if (t === 'flags') {
n = item.tags['flag:name'];
kvn = `${k}/${v}|${n}`;
Expand Down
4 changes: 3 additions & 1 deletion app/src/Header.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ function Title(props) {
let fallbackIcon;
if (t === 'brands') {
fallbackIcon = 'https://cdn.jsdelivr.net/npm/@mapbox/maki@6/icons/shop-15.svg';
} else if (t === 'denominations') {
fallbackIcon = 'https://cdn.jsdelivr.net/npm/@mapbox/maki@6/icons/place-of-worship-15.svg';
} else if (t === 'flags') {
fallbackIcon = 'https://cdn.jsdelivr.net/npm/@mapbox/maki@6/icons/embassy-15.svg';
} else if (t === 'operators') {
Expand Down Expand Up @@ -77,7 +79,7 @@ function Title(props) {

function TreeSwitcher(props) {
const t = props.t;
const others = ['brands', 'flags', 'operators', 'transit'].filter(d => d !== t);
const others = ['brands', 'denominations', 'flags', 'operators', 'transit'].filter(d => d !== t);
const links = others.map(t => (<li key={t}><Link to={`index.html?t=${t}`}>{t}/</Link></li>));
const list = links.length ? (<> see also: <ul>{links}</ul> </>) : null;

Expand Down
3 changes: 3 additions & 0 deletions app/src/Overview.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ export default function Overview(props) {
if (t === 'brands') {
fallbackIcon = 'https://cdn.jsdelivr.net/npm/@mapbox/maki@6/icons/shop-15.svg';
wikidataTag = 'brand:wikidata';
} else if (t === 'denominations') {
fallbackIcon = 'https://cdn.jsdelivr.net/npm/@mapbox/maki@6/icons/place-of-worship-15.svg';
wikidataTag = 'denomination:wikidata';
} else if (t === 'flags') {
fallbackIcon = 'https://cdn.jsdelivr.net/npm/@mapbox/maki@6/icons/embassy-15.svg';
wikidataTag = 'flag:wikidata';
Expand Down
3 changes: 3 additions & 0 deletions app/src/OverviewInstructions.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ export default function OverviewInstructions(props) {
if (t === 'brands') {
itemType = 'brand';
wikidataTag = 'brand:wikidata';
} else if (t === 'denominations') {
itemType = 'denomination';
wikidataTag = 'denomination:wikidata';
} else if (t === 'flags') {
itemType = 'flag';
wikidataTag = 'flag:wikidata';
Expand Down
9 changes: 9 additions & 0 deletions config/trees.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,15 @@
"alternate": "^(brand|brand:\\w+|operator|operator:\\w+|\\w+_name|\\w+_name:\\w+)$"
}
},
"denominations": {
"emoji": "🛐",
"mainTag": "denomination:wikidata",
"sourceTags": ["denomination"],
"nameTags": {
"primary": "^(denomination|denomination:\\w+|name|name:\\w+)$",
"alternate": "^(operator|operator:\\w+|\\w+_name|\\w+_name:\\w+)$"
}
},
"flags": {
"emoji": "🚩",
"mainTag": "flag:wikidata",
Expand Down
49 changes: 49 additions & 0 deletions data/denominations/amenity/place_of_worship.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
{
"properties": {
"path": "denominations/amenity/place_of_worship",
Copy link
Member

Choose a reason for hiding this comment

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

Besides places of worship, schools, community centers, and social facilities are often identified by their denomination as well. The editor would have to make sure not to “upgrade” something to a place of worship just because it has a matching denomination.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a good point - I was thinking about adding schools to the denominations tree but left it out for now. IIRC iD only matches items of a different category if it falls into one of these matchGroups

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I think in this situation I think it should only offer to upgrade amenity/place_of_worship, or maybe also some generically tagged features like building=yes or amenity=yes when paired with a matching name or operator.

"skipCollection": true,
"exclude": {"generic": ["^place of worship$"]}
},
"items": [
{
"displayName": "Kingdom Hall of Jehovah's Witnesses",
Copy link
Member

@1ec5 1ec5 Dec 30, 2021

Choose a reason for hiding this comment

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

Many denominations translate their names into as many local languages as possible. The appropriate name usually varies by language more than by region. As it stands, id-tagging-schema is better suited to localization than NSI. This is already an issue for the flags tree, but the issue would be more acute for any denomination tree, because the icon usually wouldn’t be very clarifying to a non-English-speaker.

Or would we limit this tree to just the denominations that have a specialized name and icon for all their places of worship? These three presets only exist because it was considered inappropriate to use the ✝️ icon for these denominations.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I was also thinking about how the displayNames for these denominations could be translated to a region's local language. I agree that id-tagging-schema would be a better place to have these presets because of these current limitations - I hadn't really thought about how the default presets are localized

Or would we limit this tree to just the denominations that have a specialized name and icon for all their places of worship?

This could work, though I can't think of any off the top of my head. I was also looking into how the denomination=mormon wiki page suggests to use operator tags to distinguish The Church of Jesus Christ of Latter-day Saints from other splinter groups. This particular case could work in NSI under the existing operator tree as localized operator values would be possible.

These three presets only exist because it was considered inappropriate to use the ✝️ icon for these denominations.

This makes sense now. I was wondering why there were only these three denomination presets

Copy link
Member

Choose a reason for hiding this comment

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

I really don't want to introduce localization into the Name Suggestion Index display names. Display name is just the label that the preset shows in RapiD and JOSM.

But I think keeping this tree similar to how the flags tree works would be ok (denomination is an identifying value, and name can be whatever people want).

Copy link
Member

Choose a reason for hiding this comment

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

But I think keeping this tree similar to how the flags tree works would be ok (denomination is an identifying value, and name can be whatever people want).

#5982 tracks making flags localizable. Maybe the solution there could inform how we’d make denomination entries localizable too. But in the meantime, id-tagging-schema at least offers a way to localize the display names, even if it doesn’t have a straightforward way to filter denominations by religion (something we’d have to solve as part of this project too).

"id": "kingdomhallofjehovahswitnesses-0274e6",
"locationSet": {"include": ["001"]},
"tags": {
"amenity": "place_of_worship",
"denomination": "jehovahs_witnesses",
"denomination:wikidata": "Q35269",
"name": "Kingdom Hall of Jehovah's Witnesses",
"religion": "christian"
}
},
{
"displayName": "La Luz del Mundo Temple",
"id": "laluzdelmundotemple-0274e6",
"locationSet": {"include": ["001"]},
"matchNames": [
"iglesia del dios vivo columna y apoyo de la verdad la luz del mundo",
"the light of the world"
],
"tags": {
"amenity": "place_of_worship",
"denomination": "la_luz_del_mundo",
"denomination:wikidata": "Q3497836",
"name": "La Luz del Mundo Temple",
"religion": "christian"
}
},
{
"displayName": "Quaker Friends Meeting House",
"id": "quakerfriendsmeetinghouse-0274e6",
"locationSet": {"include": ["001"]},
"tags": {
"amenity": "place_of_worship",
"denomination": "quaker",
"denomination:wikidata": "Q170208",
"name": "Quaker Friends Meeting House",
"religion": "christian"
}
}
]
}
8 changes: 8 additions & 0 deletions dist/filtered/denominations_discard.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"_meta": {
"version": "6.0.20220801",
"generated": "2022-08-06T21:22:19.790Z",
"url": "https://raw.githubusercontent.com/osmlab/name-suggestion-index/main/dist/filtered/denominations_discard.json",
"hash": "3bc984d771638a851ec8af8c16b3f3d6",
"collectionDate": "20220801"
},"discard": {}}
8 changes: 8 additions & 0 deletions dist/filtered/denominations_keep.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"_meta": {
"version": "6.0.20220801",
"generated": "2022-08-06T21:22:19.791Z",
"url": "https://raw.githubusercontent.com/osmlab/name-suggestion-index/main/dist/filtered/denominations_keep.json",
"hash": "e37f5545a1fdb56699f8721a6f39ca17",
"collectionDate": "20220801"
},"keep": {}}
2 changes: 2 additions & 0 deletions lib/idgen.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ export function idgen(item, tkv, locationID) {
osmtags = ['name', 'name:en', 'operator', 'operator:en', 'brand', 'brand:en', 'network', 'network:en'];
} else if (t === 'flags') {
osmtags = ['flag:name', 'subject'];
} else if (t === 'denominations') {
osmtags = ['name', 'name:en', "denomination", "operator", "operator:en"];
} else { // brands
osmtags = ['name', 'name:en', 'brand', 'brand:en', 'operator', 'operator:en', 'network', 'network:en'];
}
Expand Down
1 change: 1 addition & 0 deletions schema/categories.json
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@
],
"anyOf": [
{ "required": ["brand"] },
{ "required": ["denomination", "religion"] },
{ "required": ["flag:name", "flag:type"] },
{ "required": ["operator"] },
{ "required": ["network"] }
Expand Down
9 changes: 8 additions & 1 deletion scripts/build_index.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ let _cache = {};
loadIndex();

checkItems('brands');
checkItems('denominations');
checkItems('flags');
checkItems('operators');
checkItems('transit');
Expand Down Expand Up @@ -448,6 +449,9 @@ function mergeItems() {
if (t === 'brands') {
name = tags.brand || tags.name;

} else if (t === 'denominations') {
name = tags.denomination || tags.name;

} else if (t === 'flags') {
name = tags['flag:name'];

Expand Down Expand Up @@ -555,7 +559,7 @@ function mergeItems() {
// Copy main tag value to local tag value, but only if local value not assigned yet
// re: 6788#issuecomment-1188024213
function setLanguageTags(tags, code) {
['name', 'brand', 'operator', 'network'].forEach(k => {
['name', 'denomination', 'brand', 'operator', 'network'].forEach(k => {
const v = tags[k];
const loc_k = `${k}:${code}`; // e.g. `name:ja`
const loc_v = tags[loc_k];
Expand Down Expand Up @@ -628,6 +632,9 @@ function checkItems(t) {
case 'amenity/restaurant':
if (!tags.cuisine) { warnMissingTag.push([display(item), 'cuisine']); }
break;
case 'amenity/place_of_worship':
if (!tags.religion) { warnMissingTag.push([display(item), 'religion']); }
break;
case 'amenity/training':
if (!tags.training) { warnMissingTag.push([display(item), 'training']); }
break;
Expand Down