Skip to content

Commit 7e536c5

Browse files
committed
fix(WikidataCommand): Always process P138
The latest update for Budapest [0] complained about `data/wikidata/Q44377.json` missing. This is probably the result of the following: 1. It discovered https://www.openstreetmap.org/relation/19901264. 2. Based on its `name:etymology:wikidata` tag, it downloaded Q850862. Since we’re not interested in what the namesakes are named after, it didn’t bother about P138. 3. Later it also discovered https://www.openstreetmap.org/way/156974362. 4. It saw that Q850862 has already been downloaded, so it skipped downloading it – and it also skipped downloading its namesake, Q44377. 5. Subsequently GeoJSONCommand couldn’t find `Q44377.json`. To fix this, simply don’t skip reading back the entity and trying to extract P138 from it. This also allows moving the `file_exists()` check to `save()`, reducing code duplication. This might result in a slight performance degradation (more local disk I/O if a street with its own Wikidata item consists of multiple ways not bundled in a relation), but no increase in HTTP requests (except for now doing the one or two HTTP requests per city that used to be erronously skipped), and correctness is worth that slight performance degradation. I confirmed that running on Budapest locally, the warning is printed without the fix, and not printed with the fix. I also confirmed that with the fix, `gender.csv` correctly includes `Q44377` for Budavári alagút. Please note that while the current example doesn’t involve people, so it may seem borderline out of scope, it’s very possible for similar cases to involve people: e.g. https://www.openstreetmap.org/way/335395340 (Flórián tér overpass) could have `name:etymology:wikidata=Q65215958` (Flórián tér) – I’m not 100% sure if this would be right, but arguably yes –, and that would potentially (depending on the processing order) break the yellow color of https://www.openstreetmap.org/way/908080555. [0] https://github.com/EqualStreetNames/equalstreetnames-budapest/actions/runs/19877172828/job/56967142726
1 parent e069a6a commit 7e536c5

1 file changed

Lines changed: 20 additions & 23 deletions

File tree

Command/WikidataCommand.php

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,7 @@ function ($element): bool {
118118

119119
// Download Wikidata item
120120
$path = sprintf('%s/%s.json', $outputDir, $identifier);
121-
if (!file_exists($path)) {
122-
self::save($identifier, $element, $path, $warnings);
123-
}
121+
self::save($identifier, $element, $path, $warnings);
124122
}
125123
}
126124

@@ -134,27 +132,22 @@ function ($element): bool {
134132

135133
// Download Wikidata item
136134
$path = sprintf('%s/%s.json', $outputDir, $wikidataTag);
137-
if (!file_exists($path)) {
138-
self::save($wikidataTag, $element, $path, $warnings);
139-
140-
$wikiPath = sprintf('%s/%s.json', $outputDir, $wikidataTag);
141-
$entity = Wikidata::read($wikiPath);
142-
143-
$identifiers = Wikidata::extractNamedAfter($entity);
144-
if (!is_null($identifiers)) {
145-
foreach ($identifiers as $identifier) {
146-
// Check that the value of the tag is a valid Wikidata item identifier
147-
if (preg_match('/^Q[0-9]+$/', $identifier) !== 1) {
148-
$warnings[] = sprintf('Format of `P138` (NamedAfter) property is invalid (%s) for in item "%s".', $identifier, $wikidataTag);
149-
continue;
150-
}
151-
152-
// Download Wikidata item
153-
$path = sprintf('%s/%s.json', $outputDir, $identifier);
154-
if (!file_exists($path)) {
155-
self::save($identifier, $element, $path, $warnings);
156-
}
135+
self::save($wikidataTag, $element, $path, $warnings);
136+
137+
$entity = Wikidata::read($path);
138+
139+
$identifiers = Wikidata::extractNamedAfter($entity);
140+
if (!is_null($identifiers)) {
141+
foreach ($identifiers as $identifier) {
142+
// Check that the value of the tag is a valid Wikidata item identifier
143+
if (preg_match('/^Q[0-9]+$/', $identifier) !== 1) {
144+
$warnings[] = sprintf('Format of `P138` (NamedAfter) property is invalid (%s) for in item "%s".', $identifier, $wikidataTag);
145+
continue;
157146
}
147+
148+
// Download Wikidata item
149+
$path = sprintf('%s/%s.json', $outputDir, $identifier);
150+
self::save($identifier, $element, $path, $warnings);
158151
}
159152
}
160153
}
@@ -189,6 +182,10 @@ function ($element): bool {
189182
*/
190183
private static function save(string $identifier, $element, string $path, array &$warnings = []): void
191184
{
185+
if (file_exists($path)) {
186+
return;
187+
}
188+
192189
$url = sprintf('%s%s.json', self::URL, $identifier);
193190

194191
$retryMiddleware = Middleware::retry(

0 commit comments

Comments
 (0)