Modify

Opened 4 years ago

Closed 4 years ago

Last modified 4 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 Changed 4 years ago by naoliv

Description: modified (diff)

comment:2 Changed 4 years ago by Don-vip

Milestone: 14.12

comment:3 Changed 4 years ago by Klumbumbus

/* 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 Changed 4 years ago by Klumbumbus

Owner: changed from team to Klumbumbus
Status: newassigned

comment:5 Changed 4 years ago by Klumbumbus

In 7818/josm:

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

comment:6 Changed 4 years ago by Klumbumbus

Description: modified (diff)

comment:7 Changed 4 years ago by Klumbumbus

/* 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 Changed 4 years ago by 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.

comment:9 Changed 4 years ago by naoliv

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

comment:10 in reply to:  8 Changed 4 years ago by Klumbumbus

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 4 years ago by Klumbumbus (previous) (diff)

comment:11 in reply to:  9 Changed 4 years ago by Klumbumbus

Replying to naoliv:

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

I think there can be half height storeys.

comment:12 Changed 4 years ago by Klumbumbus

see also #10310 for shop=yes

comment:13 in reply to:  description Changed 4 years ago by Klumbumbus

/* 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 Changed 4 years ago by naoliv

The object has an old_name but not name?

comment:15 in reply to:  14 Changed 4 years ago by Klumbumbus

Replying to naoliv:

The object has an old_name but not name?

A resturant which was closed.

comment:16 Changed 4 years ago by Klumbumbus

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

comment:17 Changed 4 years ago by anonymous

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

comment:18 Changed 4 years ago by anonymous

Does not feel...

comment:19 Changed 4 years ago by Klumbumbus

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 Changed 4 years ago by naoliv

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

comment:21 in reply to:  20 Changed 4 years ago by Klumbumbus

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 Changed 4 years ago by Klumbumbus

In 7872/josm:

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

comment:23 Changed 4 years ago by Klumbumbus

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

comment:24 Changed 4 years ago by Klumbumbus

In 7876/josm:

see #10837 - fix typo

comment:25 Changed 4 years ago by Klumbumbus

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 Changed 4 years ago by Don-vip

Resolution: fixed
Status: reopenedclosed

In 7878/josm:

fix #10837 - fix asserts

comment:27 Changed 4 years ago by Don-vip

In 7880/josm:

fix #10837 - fix asserts

comment:28 Changed 4 years ago by Klumbumbus

In 7969/josm:

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

comment:29 Changed 4 years ago by mdk

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 Changed 4 years ago by Klumbumbus

In 8136/josm:

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

comment:31 Changed 4 years ago by Klumbumbus

comment:32 in reply to:  30 ; Changed 4 years ago by 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=*.

comment:33 in reply to:  32 ; Changed 4 years ago by Klumbumbus

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.

comment:34 in reply to:  33 Changed 4 years ago by skyper

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 Changed 4 years ago by Klumbumbus

In 8139/josm:

see #10837 - improve building:part preset

comment:36 Changed 4 years ago by aceman

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.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.