Opened 4 years ago
Last modified 8 months ago
#20473 new defect
Multipolygon repeating the tag of an outer way is not flagged
Reported by: | GerdP | Owned by: | team |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | Core validator | Version: | |
Keywords: | template_report multipolygon | Cc: |
Description
What steps will reproduce the problem?
- Load attched file and run validator
What is the expected result?
A warning that both the multipolygon and the outer ring have a tag leisure=water_park
What happens instead?
Only an info message
"With the currently used mappaint style the style for outer way mismatches the area style"
Please provide any additional information below. Attach a screenshot if possible.
There are probably two different cases: If there are inner rings within the outer this is an obvious error, but if there is no inner it requires expert knowledge to find out what is wrong.
Build-Date:2021-02-09 15:57:34 Revision:17488 Is-Local-Build:true Identification: JOSM/1.5 (17488 SVN en) Windows 10 64-Bit OS Build number: Windows 10 Home 2004 (19041) Memory Usage: 793 MB / 1820 MB (675 MB allocated, but free) Java version: 1.8.0_272-b10, AdoptOpenJDK, OpenJDK 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 VM arguments: [-agentlib:jdwp=transport=dt_socket,suspend=y,address=localhost:63940, -ea, -Djosm.homexxx=c:\temp\josmxxx, -javaagent:D:\eclipse-java-2020-09\eclipse\configuration\org.eclipse.osgi\577\0\.cp\lib\javaagent-shaded.jar, -Dfile.encoding=UTF-8] Program arguments: [--debug] Dataset consistency test: No problems found Plugins: + MakeParallel (v1.1.0) + buildings_tools (35669) + o5m (35640) + pbf (35650) + poly (35640) + reltoolbox (35640) + reverter (35688) + undelete (35640) + utilsplugin2 (35691)
Attachments (2)
Change History (18)
by , 4 years ago
Attachment: | 20473-sample.osm added |
---|
comment:1 by , 4 years ago
Component: | Core → Core validator |
---|---|
Keywords: | multipolygon added |
follow-up: 3 comment:2 by , 4 years ago
comment:3 by , 4 years ago
Replying to GerdP:
Please provide any additional information below. Attach a screenshot if possible.
There are probably two different cases: If there are inner rings within the outer this is an obvious error, but if there is no inner it requires expert knowledge to find out what is wrong.
+1, first difference to check.
Additionally, a difference between "primary" and "secondary" tags could be made and I am not sure what to do with "ignorable tags". We do not need a warning for automatically discarded tags and tags like note=*
, description=*
and FIXME=*
could get a different treatment.
Replying to GerdP:
I am not yet sure if the message should contain a fixed string like "Outer way has same tag as multipolygon" or if it should also contain the tag(s). If the latter, should we produce one warning for each equal tag?
I'd go with a fixed string and a warning for each MP and its members, as selecting the objects is enough to find the tags all have in common.
In many cases the multipolygons also produce other warnings like "Multipolygon tags - tag describing the area might be missing", see e.g. r2944039 (and lots of similar cases in that area). No idea if it makes sense to create even more warnings?
I am not sure, if we will be able to hide all other warnings and I am not sure if it is the best choice in all cases.
follow-up: 5 comment:4 by , 4 years ago
+1, first difference to check.
Not sure what you mean. Please elaborate.
Additionally, a difference between "primary" and "secondary" tags could be made
My first approach is to ignore the uninteresting tags and treat all others equal. I have no idea how to separate further.
I'd go with a fixed string and a warning for each MP and its members
So, only one warning per relation? Or one for each pair of MP and member? Or one for each tag? I agree that it is easy enough to find the tag when the error is double clicked so that the involved objects are highlighted.
I am not sure, if we will be able to hide all other warnings and I am not sure if it is the best choice in all cases.
I don't want to hide the existing messages for this new one, I think about suppressing the new warning if other similar messages are already produced.
comment:5 by , 4 years ago
Replying to GerdP:
Not sure what you mean. Please elaborate.
It makes a difference if the MP has members with only role outer
or also with inner
. Thinking about it again, as long as we do not want to offer an autofix, it makes no difference unless you plan to have different warning levels.
Additionally, a difference between "primary" and "secondary" tags could be made
My first approach is to ignore the uninteresting tags and treat all others equal. I have no idea how to separate further.
I was thinking about the list for the "areas" tags, in order to have different warning levels, though, that could be a future enhancement.
I'd go with a fixed string and a warning for each MP and its members
So, only one warning per relation? Or one for each pair of MP and member? Or one for each tag? I agree that it is easy enough to find the tag when the error is double clicked so that the involved objects are highlighted.
I thought about only one per relation but that does not work out nice if you have different tags on the members. The longer I think about it, the more puzzled I am. I thought selecting and having a look at the tags in common is easy but what happens if tags in question were modified/deleted in between? So I guess I am back to having the tag in the warning and have one warning per relation for each tag.
One thing I noticed, I am not using the lookup button very often in my workflow. Maybe, that would help in this case.
I am not sure, if we will be able to hide all other warnings and I am not sure if it is the best choice in all cases.
I don't want to hide the existing messages for this new one, I think about suppressing the new warning if other similar messages are already produced.
I am not feeling well, if a warning with low priority can suppress a warning on higher levels.
comment:6 by , 4 years ago
I see no way to implement an autofix.
I guess we have to limit the test to check only a specific list of tags like landuse, natural etc, else this will produce lots of rather useless messages, e.g. when a building MP has the same addr:* tags as the outer way or other descriptive tags like image=*, operator=*
comment:7 by , 4 years ago
In #19136 I've implemented a method TagChecker.hasAcceptedPrimaryTagForMultipolygon()
which contains probably the most complete list of primary tags for MP, but the message just caclulates a boolean. It doesn't return the tag / tag key.
The more samples that I look at the fewer warnings look good to me. So, the patch only warns for those tags, not for name or ref or similar.
comment:8 by , 4 years ago
I often find tags added to the outer way instead to the MP like surface=*
, level=*
or even duplicating most of the tags. I was hoping to find these problems to take a look at but it seems complicated.
comment:9 by , 4 years ago
I can add another list of tag keys as a preference to say: test these as well
comment:10 by , 4 years ago
Ok, another list in preference. I rather need an ignore (black) list than a white list, I guess.
by , 4 years ago
Attachment: | 20473.patch added |
---|
comment:13 by , 4 years ago
The patch implements the check to find when a relation has the same tag as one or more outer ways
- group "Multipolygon outer way repeats major tag of relation" in warnings
- group "Multipolygon outer way repeats tag of relation" in information for those that don't match the first
- The tag is shown with "Same tag:{0}={1}"
- preference
validator.TagChecker.ignore-keys-outer-mp-same-tag
can be used to list tag keys which should be ignored by this test
Not sure yet about the wording, it suggests that the tag on the outer way is wrong but in my example the tag has to be removed from the MP.
comment:14 by , 4 years ago
By the way: We have the mapcss test
/* see ticket #7639 -- Warn when a node has the same tags as its parent way. */ way >:sameTags node:tagged { throwWarning: tr("Nodes duplicating parent way tags"); }
This doesn't show the tag(s) but it only works when all interesting tags are the same.
I started to work on this and found lots of simiar problems like same
ref
orname
on outer way(s) and the relation.I am not yet sure if the message should contain a fixed string like "Outer way has same tag as multipolygon" or if it should also contain the tag(s). If the latter, should we produce one warning for each equal tag?
In many cases the multipolygons also produce other warnings like "Multipolygon tags - tag describing the area might be missing", see e.g. r2944039 (and lots of similar cases in that area). No idea if it makes sense to create even more warnings?