Modify

Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#10837 closed enhancement (fixed)

Some more validation rules for JOSM

Reported by: naoliv Owned by: Klumbumbus
Priority: normal Milestone: 14.12
Component: Core validator Version:
Keywords: mapcss Cc:

Description (last modified by Klumbumbus)

Good that you have included #10825 :-)
I have some more rules that we are using here that probably could be used everywhere else (and maybe you could be interested in including some of them in JOSM)

/* is using any other *_name tag but not a name */
*[/_name$/][!name] {
        throwWarning: tr("using alternative name without {0}", "{1.key}");
}

Done.

/* using nomame = yes + name is contradictory */
*[noname?][name] {
        throwWarning: tr("contradictory use of {0} and {1}", "{0.key}", "{1.key}");
}

Done.

/* follows http://wiki.openstreetmap.org/wiki/Proposed_features/drop_recommendation_for_place_name */

/* when name = place_name we just remove place_name */
*[place][place_name = *name] {
        throwWarning: tr("{0} = {1}; remove {0}", "{1.key}", "{1.value}");
        fixRemove: "{1.key}";
}

/* when it has place_name but not name, we rename place_name to name */
*[place][place_name][!name] {
        throwWarning: tr("do not use {0}", "{1.key}");
        suggestAlternative: "name";
        fixChangeKey: "{1.key} => {2.key}";
}

Done.

/* the highway "name" already define it's own address; it's strange to also have a "addr:street" on highways */
*[highway]["addr:street"] {
        throwWarning: tr("{0} should not have a {1} tag", "{0.key}", "{1.key}");
}

Done.

/* should use building:levels instead building + levels */
*[building][levels] {
        throwWarning: tr("use building:levels instead {0}", "{1.key}");
        fixChangeKey: "levels => building:levels";
}

Done.

/* they shouldn't have a "yes" or "true" value */
*[amenity?],
*[place?] {
        throwWarning: tr("unspecific value for {0}", "{0.key}");
}

Done.

/* lanes* must be an integer positive number only */
way[highway][lanes][lanes !~ /^[1-9]([0-9]*)$/],
way[highway]["lanes:backward"]["lanes:backward" !~ /^[1-9]([0-9]*)$/],
way[highway]["lanes:forward"]["lanes:forward" !~ /^[1-9]([0-9]*)$/] {
        throwError: tr("{0} must be a positive integer number", "{1.key}");
}

Done.

/* destination rules http://wiki.openstreetmap.org/wiki/Key:destination */
/* destination is "Worthwile only if used in combination with oneway=yes" */
way[destination][!oneway?] {
        throwWarning: tr("incomplete usage of {0}", "{0.key}");
        suggestAlternative: "destination:forward";
        suggestAlternative: "destination:backward";
}

Done.

Note that those tests were created for data that we were seeing here in Brazil but they aren't specific for here.
Some warning messages probably need a better wording.

Anything that could be useful for JOSM?

Attachments (0)

Change History (36)

comment:1 by naoliv, 9 years ago

Description: modified (diff)

comment:2 by Don-vip, 9 years ago

Milestone: 14.12

comment:3 by Klumbumbus, 9 years ago

/* when it has place_name but not name, we rename place_name to name */
*[place][place_name][!name] {
        throwWarning: tr("do not use {0}", "{1.key}");
        suggestAlternative: "name";
        fixChangeKey: "{1.key} => {2.key}";
}

This is already included. See [7697]

comment:4 by Klumbumbus, 9 years ago

Owner: changed from team to Klumbumbus
Status: newassigned

comment:5 by Klumbumbus, 9 years ago

In 7818/josm:

see #10837 - add some more validation rules (based on rules by naoliv)

comment:6 by Klumbumbus, 9 years ago

Description: modified (diff)

comment:7 by Klumbumbus, 9 years ago

/* lanes* must be an integer positive number only */
way[highway][lanes][lanes !~ /^[1-9]([0-9]*)$/],
way[highway]["lanes:backward"]["lanes:backward" !~ /^[1-9]([0-9]*)$/],
way[highway]["lanes:forward"]["lanes:forward" !~ /^[1-9]([0-9]*)$/] {
        throwError: tr("{0} must be a positive integer number", "{1.key}");
}

@team: is this fine in mapcss or better handled in a java validator test?

comment:8 by aceman, 9 years ago

Hi, what about shop=yes ? I see it is used about 30000 times and has even a wiki page. But we could have an 'info' or 'warning' message about it. It seems discouraged to use it.

comment:9 by naoliv, 9 years ago

Should building:levels also be tested for positive integer numbers?

in reply to:  8 comment:10 by Klumbumbus, 9 years ago

Replying to aceman:

Hi, what about shop=yes ? I see it is used about 30000 times and has even a wiki page. But we could have an 'info' or 'warning' message about it. It seems discouraged to use it.

It's often used at amenity=fuel. It's even in the default preset.

edit: but I agree, this is not optimal

Last edited 9 years ago by Klumbumbus (previous) (diff)

in reply to:  9 comment:11 by Klumbumbus, 9 years ago

Replying to naoliv:

Should building:levels also be tested for positive integer numbers?

I think there can be half height storeys.

comment:12 by Klumbumbus, 9 years ago

see also #10310 for shop=yes

in reply to:  description comment:13 by Klumbumbus, 9 years ago

/* is using any other *_name tag but not a name */
*[/_name$/][!name] {
        throwWarning: tr("using alternative name without {0}", "{1.key}");
}

This test produces false positives for:

  • old_name
  • loc_name
  • uic_name

comment:14 by naoliv, 9 years ago

The object has an old_name but not name?

in reply to:  14 comment:15 by Klumbumbus, 9 years ago

Replying to naoliv:

The object has an old_name but not name?

A resturant which was closed.

comment:16 by Klumbumbus, 9 years ago

Or an old industrial building which was abandoned and not yet reused.

comment:17 by anonymous, 9 years ago

Does feel to me like the right usage of old_name. Maybe disused:name=* would be better.

comment:18 by anonymous, 9 years ago

Does not feel...

comment:19 by Klumbumbus, 9 years ago

way/286295915 was part of the main street "Zschopauer Straße" before the trunk was built in the last years. Now it has become just a driveway without a name.

comment:20 by naoliv, 9 years ago

Since it's just a warning (and not an error), can't it be ignored by the user?

in reply to:  20 comment:21 by Klumbumbus, 9 years ago

Replying to naoliv:

Since it's just a warning (and not an error), can't it be ignored by the user?

The most messages are warning level. And if there are too much false positives, people don't trust the validator and start ignoring the whole validator.

comment:22 by Klumbumbus, 9 years ago

In 7872/josm:

see #10837 - add numeric lanes validation rule (rule by naoliv); fix names validation rule

comment:23 by Klumbumbus, 9 years ago

Description: modified (diff)
Resolution: fixed
Status: assignedclosed

comment:24 by Klumbumbus, 9 years ago

In 7876/josm:

see #10837 - fix typo

comment:25 by Klumbumbus, 9 years ago

Resolution: fixed
Status: closedreopened

The AssertMatches are failing. I don't know whats wrong there.

http://donvip.fr/jenkins/job/JOSM/1696/testReport/org.openstreetmap.josm.data.validation.tests/MapCSSTagCheckerTest/testInit/

I think the regexp can be simplified to ^[1-9]\d*$, but I don't know if this helps.

comment:26 by Don-vip, 9 years ago

Resolution: fixed
Status: reopenedclosed

In 7878/josm:

fix #10837 - fix asserts

comment:27 by Don-vip, 9 years ago

In 7880/josm:

fix #10837 - fix asserts

comment:28 by Klumbumbus, 9 years ago

In 7969/josm:

see #10837 - warn about levels=* also for building:part=yes

comment:29 by mdk, 9 years ago

If you suggest keys like building:levels, the validator should not complain with:
Key building:levels not in presets. - Presets do not contain property key.
Same with building:part.

comment:30 by Klumbumbus, 9 years ago

In 8136/josm:

see #10837 - add building:part=yes, building:levels, height, building:min_level and min_height

comment:31 by Klumbumbus, 9 years ago

in reply to:  30 ; comment:32 by skyper, 9 years ago

Replying to Klumbumbus:

In 8136/josm:

see #10837 - add building:part=yes, building:levels, height, building:min_level and min_height

Think it should be building:part=* with same values as building=*.

in reply to:  32 ; comment:33 by Klumbumbus, 9 years ago

Replying to skyper:

Replying to Klumbumbus:

In 8136/josm:

see #10837 - add building:part=yes, building:levels, height, building:min_level and min_height

Think it should be building:part=* with same values as building=*.

Wiki and taginfo don't say this.

in reply to:  33 comment:34 by skyper, 9 years ago

Replying to Klumbumbus:

Replying to skyper:

Think it should be building:part=* with same values as building=*.

Wiki and taginfo don't say this.

Right, it is only mention under Simple 3D Buildings. Taginfo is a bit difficult to interpret as e.g. building=roof is special and if using aerials you often have to add several parts for one building and it is often not possible to determine the right value as even with a proper value of building=* the value for the part might be different.

Miss building:part=balcony.

comment:35 by Klumbumbus, 9 years ago

In 8139/josm:

see #10837 - improve building:part preset

comment:36 by aceman, 9 years ago

Yeas I also agree and use building:part=<all the values from key:building>. Thanks for the fix.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Klumbumbus.
as The resolution will be set.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.