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.

Sign up for GitHub

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

define a schema for relation members #174

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

define a schema for relation members #174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
26 changes: 24 additions & 2 deletions lib/build.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import chalk from 'chalk';
import fs from 'fs';
import { globSync } from 'glob';
import jsonschema from 'jsonschema';
import { Validator } from 'jsonschema';
import path from 'path';
import shell from 'shelljs';
import YAML from 'js-yaml';
Expand All @@ -21,6 +21,8 @@ const discardedSchema = require('../schemas/discarded.json');

let _currBuild = null;

const jsonschema = new Validator();

function validateData(options) {
const START = '🔬 ' + chalk.yellow('Validating schema...');
const END = '👍 ' + chalk.green('schema okay');
Expand Down Expand Up @@ -203,6 +205,9 @@ function read(f) {


function validateSchema(file, instance, schema) {
// add this schema to the cache, so $ref can be resolved faster
jsonschema.addSchema(schema);

let validationErrors = jsonschema.validate(instance, schema).errors;

if (validationErrors.length) {
Expand Down Expand Up @@ -362,6 +367,15 @@ function generatePresets(dataDir, tstrings, searchableFieldIDs, listReusedIcons)
if (!icons[icon]) icons[icon] = [];
icons[icon].push(id);
}

if (preset.relation) {
tstrings.presets[id].relation_roles = {};
for (const member of preset.relation.members) {
for (const role in member.roles) {
tstrings.presets[id].relation_roles[role] = member.roles[role];
}
}
}
});

if (listReusedIcons) {
Expand Down Expand Up @@ -454,8 +468,10 @@ function generateTranslations(fields, presets, tstrings, searchableFieldIDs) {
let tags = preset.tags || {};
let keys = Object.keys(tags);

const tagsString = keys.map(k => `${k}=${tags[k]}`).join(' + ');

if (keys.length) {
yamlPreset['#name'] = keys.map(k => `${k}=${tags[k]}`).join(' + ');
yamlPreset['#name'] = tagsString;
if (yamlPreset.aliases) {
yamlPreset['#name'] += ' | ' + yamlPreset.aliases.split('\n').join(', ');
}
Expand All @@ -466,6 +482,12 @@ function generateTranslations(fields, presets, tstrings, searchableFieldIDs) {
yamlPreset['#name'] += ` | Local preset for countries ${preset.locationSet.include.map(country => `"${country.toUpperCase()}"`).join(', ')}`;
}

if (yamlPreset.relation_roles) {
for (const role in yamlPreset.relation_roles) {
yamlPreset.relation_roles[`#${role}`] = `Relation role “${role}” when used with ${tagsString}`;
}
}

if (preset.searchable !== false) {
if (yamlPreset.terms) {
yamlPreset['#terms'] = 'terms: ' + yamlPreset.terms;
Expand Down
11 changes: 8 additions & 3 deletions schemas/field.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@
"minItems": 1,
"uniqueItems": true,
"items": {
"type": "string",
"enum": ["point", "vertex", "line", "area", "relation"]
"$ref": "#/$defs/Geometry"
}
},
"default": {
Expand Down Expand Up @@ -294,5 +293,11 @@
{ "required": ["keys"] }
]}
]}
]
],
"$defs": {
"Geometry": {
"type": "string",
"enum": ["point", "vertex", "line", "area", "relation"]
}
}
}
82 changes: 81 additions & 1 deletion schemas/preset.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,88 @@
}
},
"additionalProperties": false
},
"relation": {
"$ref": "#/$defs/RelationSchema"
},
"relationCrossReference": {
"description": "A preset can reference the relation schema from another preset",
"type": "string",
"pattern": "^\\{.+\\}$"
}
},
"additionalProperties": false,
"required": ["name", "geometry", "tags"]
"required": ["name", "geometry", "tags"],
"$defs": {
"RelationSchema": {
"type": "object",
"properties": {
"optionalTags": {
"type": "object",
"description": "Only useful for specifying placeholders which are referenced in members.*.matchTags",
"examples": [{ "route": "$1" }],
"additionalProperties": {
"type": "string"
}
},
"id": {
"type": "string",
"description": "The “permanent relation type ID”, this should match the value of https://osm.wiki/Property:P41 in the OSM wiki’s wikibase system."
},
"allowDuplicateMembers": {
"type": "boolean",
"default": true
},
"members": {
"type": "array",
"items": {
"type": "object",
"properties": {
"roles": {
"type": "object",
"additionalProperties": {
"type": "string"
},
"description": "Map of roles to their label in the default language. An empty string is allowed as key."
Copy link

Choose a reason for hiding this comment

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

How useful is it to be able to combine multiple roles in the same role definition? Are there any relation types with lots of very similar roles that would have verbose definitions?

Copy link
Collaborator Author

@k-yle k-yle Dec 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

PTv2's stop(_on_demand)?(_exit_only|_entry_only)? is probably the most verbose (likewise for platform_*). That would be 6 entries in a single JSON block.

So, there would be 13+ translatable strings for the PTv2 schema. I'm not sure how we could optimise this further

"members": [
 {}, // 6x stop_*                vertex
 {}, // 6x platform_*            line + area
 {}, // 1x (empty string)        line
 {}, // 0x (other)               any geometry
]

Copy link

Choose a reason for hiding this comment

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

Ah OK, that’s what I was missing – a case where a relation type has lots of very similar roles.

},
"geometry": {
"type": "array",
"items": {
"$ref": "field.json#/$defs/Geometry"
},
"description": "If not specified, any geometry is allowed"
},
"matchTags": {
Copy link

Choose a reason for hiding this comment

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

Would it be useful to define something for documentation here too? It would be nice to be able to show a tooltip with a description of the role, for example to clarify that “Capital” (admin_centre) is a capital city, not a government headquarters building. The matchTags should prevent further mixups, but the user might encounter an already mistagged role and need to know the difference.

The wiki has a number of data items about roles, but none of them are tied to article titles. There’s a separate set of articles about roles under the “Role:” pseudonamespace, but these mostly serve as disambiguation pages, directing readers to the relation type pages.

We could set up more specific, per–relation type pages that have predictable titles and are linked to the data items. If so, then we wouldn’t necessarily need anything about documentation in the schema. Alternatively, we could add a property to the schema that indicates the QID of the data item about the role in the particular relation type.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

great question, I wasn't aware of these data items! It's a shame that we can't query the wikibase API by P21 permanent relation role ID

What approach do you think is better? I can see downsides of both methods:

  • Creating wiki pages like Role:multipolygon=outer might confuse people because the syntax for P21 looks very similar to an OSM tag. Role:multipolygon/outer might be less confusing. I assume these pages would be stubs or redirect pages?
  • Storing the QID for each role in this repository would be cumbersome and error-prone

Copy link

@1ec5 1ec5 Dec 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

great question, I wasn't aware of these data items! It's a shame that we can't query the wikibase API by P21 permanent relation role ID

openstreetmap/operations#903 would install an extension that would allow us to easily query for data items by the value of a statement. In theory, you can already do so using Sophox, but whatever process ingests new data items into Sophox hasn’t been working for a while: Sophox/sophox#31.

I assume these pages would be stubs or redirect pages?

I was thinking of creating them as redirect pages for now, since there are already other pages that go into more detail, but in principle there could be full-fledged pages in the future.

Creating wiki pages like Role:multipolygon=outer might confuse people because the syntax for P21 looks very similar to an OSM tag.

Or maybe “Relation:multipolygon/outer”, assuming a relation type will never have a slash in it. Both this format and “Role:multipolygon/outer” carry the downside that, if the Translate extension is ever installed on the wiki (openstreetmap/operations#649), it’ll start using subpages for translations, with ISO language codes as subpage names. The to role will conflict with Tongan, maybe others too. Blank roles would also wind up with an awkward page title. Maybe something more verbose but less ambiguous like “Relation:multipolygon/Roles/outer” or “Role:outer (multipolygon)” would be more future-proof. We could ask the wiki community what they’d prefer.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

openstreetmap/operations#903 seems like the simplest solution to me, since it doesn't involve mass-creating wiki pages, nor inventing a new syntax for these pages' names.

The only thing we would need to store in this repository is the P41 for each relation. Then we can lookup the documentation for each role using the same approach as presets & fields.

I'm not very familiar with mediawiki, but we're running v1.39, so there's no major update required to install this extension?

Copy link

Choose a reason for hiding this comment

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

There aren’t any technical blockers as far as I know. But that request has been languishing for a while, so it might be quicker to just create the redirects on the wiki as a stopgap.

Copy link

Choose a reason for hiding this comment

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

For what it's worth, following a request on Talk:Wiki that cited this discussion, I added data item Descriptions for two dozen most common relation roles. Only a few are linked to Wiki articles proper (mostly unusable), such as Q22741 and a few I added are linked to redirects (such as Q22737 ).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've added a property called relation.id to the schema, which would store the P41 value, such as multipolygon. This will be needed regardless of how we query the wiki.

[...] We could ask the wiki community what they’d prefer.

It sounds like creating redirect or stub pages for each role isn't a popular idea.... Which means there's no way for iD to query this data without the WikibaseCirrusSearch extension.

Copy link

Choose a reason for hiding this comment

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

Using the new relation member roles (P54) property, the existing wbgetentities response now includes the QID of the item for each role pertaining to the relation type. We can get the ID and descriptions of each item in a separate wbgetentities call. This isn’t as streamlined as the redirect approach, but it should still be workable, since none of the relation types has more than a handful of itemized roles.

Alternatively, if we’re comfortable with relying on the apparent English label naming convention for these role items, we can issue a single wbsearchentities call in the form “type relation” to get the relation type item plus all the associated role items. However, this is inherently limited to getting the description in the user’s language; we wouldn’t be able to get any image or other metadata.

Copy link

Choose a reason for hiding this comment

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

I also just noticed that a bunch of the role items already have their sitelinks set to nonexistent pages like “Relation:boundary=admin centre”, as documented. This makes it possible to access these items directly using wbgetentities.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

in that case, I believe that https://github.com/ideditor/schema-builder/compare/5157ab..31f9af means that the schema now has all the information required to construct an API query for the documentation (?)

"type": "array",
"items": {
"type": "object",
"additionalProperties": {
"type": "string"
}
},
"examples": [
[{}],
[{ "a": 1, "b": 2 }],
[{ "a": 1 }, { "b": 2 }]
],
"description": "`*` can be used as a tag value. If multiple array items are specified, only 1 needs to match."
},
"min": {
"type": "integer",
"description": "If unspecified, there is no minimum"
},
"max": {
"type": "integer",
"description": "If unspecified, there is no maximum"
}
},
"required": ["matchTags"],
"additionalProperties": false
}
}
},
"required": ["id", "allowDuplicateMembers", "members"],
"additionalProperties": false
}
}
}
Loading