Modify

Opened 4 months ago

Closed 4 months ago

Last modified 3 months ago

#16188 closed enhancement (fixed)

[PATCH] Add support for crossing of residential areas with something.

Reported by: marxin Owned by: team
Priority: normal Milestone: 18.04
Component: Core validator Version: latest
Keywords: Klumbumbus Cc: Klumbumbus

Description

The patch makes detection of area more generic and adds support for residential areas.

Attachments (1)

0001-Add-support-for-crossing-of-residential-areas-with-s.patch (7.4 KB) - added by marxin 4 months ago.
V3 of the patch

Download all attachments as: .zip

Change History (17)

comment:1 Changed 4 months ago by Don-vip

Component: CoreCore validator
Keywords: Klumbumbus added

Thanks for the patch!
I see some issues to fix before merging:

  • the removal of mapcss rule means there are several use cases no longer detected (for example, two crossing industrial landuses). It should rather skip the residential value.
  • the translations using string concatenation (tr("Crossing " + types[0] + "s")) won't work. The text to translate is statically extracted from source code. Full strings must be used.

comment:2 in reply to:  1 ; Changed 4 months ago by marxin

Replying to Don-vip:

Thanks for the patch!
I see some issues to fix before merging:

  • the removal of mapcss rule means there are several use cases no longer detected (for example, two crossing industrial landuses). It should rather skip the residential value.

Can you please help me how to exclude landuse=residential in the mapcss. I'm not much familiar with that.

  • the translations using string concatenation (tr("Crossing " + types[0] + "s")) won't work. The text to translate is statically extracted from source code. Full strings must be used.

Oh, you are right. I'm sending updated patch, but it doesn't scale much as one needs to explicitly handle all N*(N-1) possible values.

comment:3 in reply to:  2 ; Changed 4 months ago by Don-vip

Cc: Klumbumbus added

Replying to marxin:

Can you please help me how to exclude landuse=residential in the mapcss. I'm not much familiar with that.

Probably something like that:

#mapcss
area:closed:areaStyle[landuse!=residential][tag("landuse") = parent_tag("landuse")] ⧉ area:closed:areaStyle[landuse!=residential]

MapCSS expert in copy :)

comment:4 in reply to:  3 ; Changed 4 months ago by Klumbumbus

OK, now that you added me in keywords and cc you got my attention ;)

Replying to Don-vip:

Probably something like that:

area:closed:areaStyle[landuse!=residential][tag("landuse") = parent_tag("landuse")]  area:closed:areaStyle[landuse!=residential]

Yes, simply add [landuse!=residential] to exclude this tag, but I think you forgot/overwrote the last [landuse] at the end of the selector:

area:closed:areaStyle[landuse!=residential][tag("landuse") = parent_tag("landuse")]  area:closed:areaStyle[landuse!=residential][landuse]

comment:5 in reply to:  4 ; Changed 4 months ago by Don-vip

Replying to Klumbumbus:

OK, now that you added me in keywords and cc you got my attention ;)

Haha sorry I often do the mistake.

I think you forgot/overwrote the last [landuse] at the end of the selector:

This was intentional. [landuse!=residential] implies we have a landuse value, different from residential, right?

comment:6 in reply to:  5 Changed 4 months ago by Klumbumbus

Replying to Don-vip:

[landuse!=residential] implies we have a landuse value, different from residential, right?

No, e.g. way[landuse!=residential]{} matches a way tagged (only) with natural=wood

Version 0, edited 4 months ago by Klumbumbus (next)

comment:7 Changed 4 months ago by marxin

So can you please provide a final version of the mapcss change.
I'm bit confused which selector is the right one?

comment:8 Changed 4 months ago by Klumbumbus

If I understand the changed Java test right, then the mapcss selector should change to:

area:closed:areaStyle[landuse!=residential][tag("landuse") = parent_tag("landuse")]  area:closed:areaStyle[landuse!=residential][landuse]

Changed 4 months ago by marxin

V3 of the patch

comment:9 in reply to:  8 ; Changed 4 months ago by marxin

Replying to Klumbumbus:

If I understand the changed Java test right, then the mapcss selector should change to:

area:closed:areaStyle[landuse!=residential][tag("landuse") = parent_tag("landuse")]  area:closed:areaStyle[landuse!=residential][landuse]

Thanks for that. I verified it works for e.g. industrial landuses. Due to that I found one issue in my patch:

p.get("landuse") == "residential"

is different from

p.hasKey("landuse", "residential")

I'm attaching new version of the patch.

Last edited 4 months ago by Don-vip (previous) (diff)

comment:10 in reply to:  9 Changed 4 months ago by Don-vip

Replying to marxin:

p.get("landuse") == "residential"

is different from

p.hasKey("landuse", "residential")

Yes the good call is:

p.hasTag("landuse", "residential")

p.hasKey("landuse", "residential") means p has landuse=* or residential=*

Last edited 4 months ago by Don-vip (previous) (diff)

comment:11 Changed 4 months ago by Don-vip

Resolution: fixed
Status: newclosed

In 13671/josm:

fix #16188 - Detect crossing of residential areas (patch by marxin, modified)

comment:12 Changed 4 months ago by Don-vip

Thanks for the patch! I have modified it a little bit to replace the string by an enum. I didn't test it much so I hope I didn't break anything.

comment:13 Changed 4 months ago by Don-vip

In 13678/josm:

see #16188 - rework new test

comment:14 Changed 3 months ago by Don-vip

Milestone: 18.04

comment:15 Changed 3 months ago by Don-vip

With #16264 I agree: we shouldn't raise a warning when a waterway crosses a residential area. This is quite common.

comment:16 Changed 3 months ago by Don-vip

In 13721/josm:

fix #16264, see #16188, see #16189 - reduce validator false positives:

  • increase tolerance of buildings with almost right angle to 1 degree
  • detect crossing residential areas only with themselves and buildings
  • fix crossing railways messages

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
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.