Opened 3 years ago
Last modified 3 years ago
#21332 new enhancement
Role verification: Always list parent relation in addition to members
Reported by: | zstadler | Owned by: | team |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | Core validator | Version: | tested |
Keywords: | template_report relation member role validation | Cc: |
Description (last modified by )
What steps will reproduce the problem?
- Click "File -> Download object..." -> relation 3472179 + download referrers + download relation members
- Click "Validation" -> check there are no "Role verification problem" warnings.
- Click "Download" -> use current map extent and download all OSM data
- Click "Validation" again -> find a new "Role verification problem" for relation 3472179 - "Haifa Trail - Downtown".
- Confirm that all members of this route relation have a "highway" tag using the OSM website or this Overpass-Turbo query.
What is the expected result?
- When a "Role verification problem" warning is found, it should include both two objects involved: the parent relation and the member in order to allow examination. This would be similar to other warning, such as highway crossings.
- If the warning in step 4 is valid, it should have been presented in step 2.
- Walking and hiking relations may have members that are relations. If the warning in step 4 is related to relation 13207704 "Haifa Trail" and its member relation 3472179 "Haifa Trail - Downtown", then the rule should be restricted to members that are way elements. See the OSM wiki on Walking Routes.
What happens instead?
- A new warning is added in step 4 above:
route("Haifa Trail - Downtown", 62 members) Role verification problem Role of relation member does not match template expression 'highway || route=ferry' in preset Hiking Route Test: Relation checker Java: org.openstreetmap.josm.data.validation.tests.RelationChecker
- It is not clear why this warning is given.
- It is not clear why the warning was not given in step 2.
Please provide any additional information below. Attach a screenshot if possible.
URL:https://josm.openstreetmap.de/svn/trunk Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b Last:Changed Date: 2021-09-03 03:12:33 +0200 (Fri, 03 Sep 2021) Build-Date:2021-09-03 01:31:19 Revision:18193 Relative:URL: ^/trunk Identification: JOSM/1.5 (18193 en_GB) Windows 10 64-Bit OS Build number: Windows 10 Pro 2009 (22000) Memory Usage: 599 MB / 3612 MB (86 MB allocated, but free) Java version: 1.8.0_301-b09, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM Look and Feel: com.sun.java.swing.plaf.windows.WindowsLookAndFeel Screen: \Display0 1920×1080 (scaling 1.00×1.00) Maximum Screen Size: 1920×1080 Best cursor sizes: 16×16→32×32, 32×32→32×32 System property file.encoding: Cp1252 System property sun.jnu.encoding: Cp1252 Locale info: en_GB Numbers with default locale: 1234567890 -> 1234567890 VM arguments: [-Djava.security.manager, -Djava.security.policy=file:<java.home>\lib\security\javaws.policy, -DtrustProxy=true, -Djnlpx.home=<java.home>\bin, -Djnlpx.origFilenameArg=%UserProfile%\AppData\LocalLow\Sun\Java\Deployment\cache\6.0\56\1ee8cfb8-708180f6, -Djnlpx.remove=false, -Djava.util.Arrays.useLegacyMergeSort=true, -Djnlpx.splashport=58381, -Djnlp.application.href=https://josm.openstreetmap.de/download/josm.jnlp, -Djnlpx.jvm=<java.home>\bin\javaw.exe] Dataset consistency test: No problems found Plugins: + apache-commons (35524) + contourmerge (v0.1.8) + ejml (35458) + geotools (35458) + importvec (35567) + jaxb (35543) + jts (35458) + opendata (35803) + reverter (35732) + undelete (35640) + utilsplugin2 (35792) Last errors/warnings: - 01070.374 E: Failed to locate image 'https://israelhiking.osm.org.il/content/favicons/favicon.ico' - 01070.377 E: Failed to locate image 'https://israelhiking.osm.org.il/content/favicons/favicon.ico' - 02233.457 E: Failed to locate image 'https://israelhiking.osm.org.il/content/favicons/favicon.ico' - 02233.459 E: Failed to locate image 'https://israelhiking.osm.org.il/content/favicons/favicon.ico'
Attachments (4)
Change History (22)
by , 3 years ago
Attachment: | JOSM_RelationChecker.png added |
---|
comment:2 by , 3 years ago
Component: | Core → Internal preset |
---|---|
Description: | modified (diff) |
Keywords: | relation member role validation added |
Version: | → tested |
by , 3 years ago
Attachment: | relationMembers.png added |
---|
comment:3 by , 3 years ago
All members of the relation 3472179 have highway tags with different values:
Even if you change them all of them to the same value (like highway=residential
), the warning still exists.
But if you delete the master relation 13207704, the warning disappear. I have no clue, why the master relation has impact in this case!
follow-up: 7 comment:5 by , 3 years ago
Replying to zstadler <zeev.stadler@…>:
What is the expected result?
- When a "Role verification problem" warning is found, it should include both two objects involved: the parent relation and the member in order to allow examination. This would be similar to other warning, such as highway crossings.
+1, yes, this would be an enhancement
- If the warning in step 4 is valid, it should have been presented in step 2.
I guess, the relation was still selected as "Download object" does this. This means that validator was only run on the selection and there are no problems with the "Downtown" part.
- Walking and hiking relations may have members that are relations. If the warning in step 4 is related to relation 13207704 "Haifa Trail" and its member relation 3472179 "Haifa Trail - Downtown", then the rule should be restricted to members that are way elements. See the OSM wiki on Walking Routes.
The proposal for the new recreation route relation roles is missing the introduction of route relations as members. In my eyes, a different type=*
should be used for such parent relations. We already have superroute
and route_master
and the first should work in your example.
JOSM checks on the object type and tags of members.
See also my comment on #20731.
comment:6 by , 3 years ago
Description: | modified (diff) |
---|
comment:7 by , 3 years ago
Replying to skyper:
Replying to zstadler <zeev.stadler@…>:
- If the warning in step 4 is valid, it should have been presented in step 2.
I guess, the relation was still selected as "Download object" does this. This means that validator was only run on the selection and there are no problems with the "Downtown" part.
Yes, you are right.
- Walking and hiking relations may have members that are relations. If the warning in step 4 is related to relation 13207704 "Haifa Trail" and its member relation 3472179 "Haifa Trail - Downtown", then the rule should be restricted to members that are way elements. See the OSM wiki on Walking Routes.
The proposal for the new recreation route relation roles is missing the introduction of route relations as members. In my eyes, a different
type=*
should be used for such parent relations. We already havesuperroute
androute_master
and the first should work in your example.
JOSM checks on the object type and tags of members.
See also my comment on #20731.
I'm not looking for modifying this route as I'm not familiar with it. I did, however, leave a discussion comment to the author.
In my eyes, this use of a relation of walking relations is inline with the general use of relations and is explicitly written in the Walking Routs wiki page. If and when this use will be deprecates, the check can and should be changed back to its current logic.
by , 3 years ago
by , 3 years ago
Attachment: | afterDelete.png added |
---|
comment:8 by , 3 years ago
Replying to GerdP:
Isn't the warning given for the master relation?
I don't think so. There are two relations, but the warning shows the detail relation (with 62 members):
After deleting the master relation and validating again, the warning disappears:
Or is the warning for the master relation and a missing role for the detail relation? And the warning message is showing the detail (relation)? But this would be very misleading! According to the warning I would expect the problem in the detail relation (with 62 members) and not in the (nowhere mentioned) parent relation.
comment:9 by , 3 years ago
The warning is given for the master relation. The problem is that the error text only mentions the member.
comment:10 by , 3 years ago
You was faster than my edit :-)
But in the end it looks like unclear warning message.
comment:11 by , 3 years ago
Yes, esp. because a right click on the relation opens the member, not the relation which is wrong. Maybe the text could be improved by adding "listed"?
Role of relation member does not match template expression
-> Role of listed relation member does not match template expression
comment:12 by , 3 years ago
I was specially confused about opening the detail relation to fix the problem. When the master relation would be opened in that case, it would be much clearer.
comment:13 by , 3 years ago
The elemnt that is currently shown in the warning details is the member, not the containing relation.
Usually, it is a way member of the route relation and this member's tags do not comply with the check's requierments.
In this specific case, the "offending" member shown was a route relation by itself, and that caused the confusion.
comment:14 by , 3 years ago
Let's clarify this:
- This test is about offending members and the members are listed
- The list is summarized as members are only listed once despite being a problem in several relations
- for incomplete members the relation itself is listed
It would be useful to get some information about the relations of which the listed member is a problem. Right now, I need to select the listed members and take a look at their memberships to find the relations I have to modify.
I am not sure what makes more sense in grouping but either the relation and all offending members or the member and all relations it is a problem in should be listed which would add one more level in the tree.
comment:15 by , 3 years ago
I suggest that:
- Only way members of a walking/hiking route will considered for the check. Relation members and node members would be ignored.
- The phrasing of the warning will be modified to something like
A way member of a walking route is not tagged as 'highway || route=ferry'
In such a case:
- No need to give reference the containing relation. The word "role" in the current phrasing is misleading. The "role" of the member in the parent relation has nothing to do with this warning.
- Providing a single copy of each member way makes a lot of sense since each of them can and should be handled once by either correcting its own tags or ignoring it.
In any case, it does not make sense to me that a node or a relation member needs to have a "highway" or a "route=ferry" tag.
comment:16 by , 3 years ago
The java test is based on the data in the presets. Not sure if the java code can be changed like that.
comment:17 by , 3 years ago
Component: | Internal preset → Core validator |
---|
This is a problem in validator and cannot be fixed in the presets.
If the member is incomplete, the parent relation is listed. But if the member is complete, only the member is listed which in this case is also a relation which leads to more confusion.
Additionally, in the example, the parent relation is wrongly tagged as type=route
but should be type=superroute
, type=collection
or even deleted as it only contains one member, atm.
This test checks that the correct role is used with the correct object type and tags. In walking routes only ways with certain tags are allowed with empty role, so the warning is correct.
But the question is what objects should be listed and in which order.
comment:18 by , 3 years ago
Summary: | False positive from RelationChecker? → Role verification: Always list parent relation in addition to members |
---|---|
Type: | defect → enhancement |
screenshot