Modify

Opened 6 years ago

Closed 6 years ago

Last modified 6 years 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 6 years ago.
V3 of the patch

Download all attachments as: .zip

Change History (17)

comment:1 by Don-vip, 6 years ago

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.

in reply to:  1 ; comment:2 by marxin, 6 years ago

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.

in reply to:  2 ; comment:3 by Don-vip, 6 years ago

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 :)

in reply to:  3 ; comment:4 by Klumbumbus, 6 years ago

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]

in reply to:  4 ; comment:5 by Don-vip, 6 years ago

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?

in reply to:  5 comment:6 by Klumbumbus, 6 years ago

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 and an untagged way too

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

comment:7 by marxin, 6 years ago

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

comment:8 by Klumbumbus, 6 years ago

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]

in reply to:  8 ; comment:9 by marxin, 6 years ago

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 6 years ago by Don-vip (previous) (diff)

in reply to:  9 comment:10 by Don-vip, 6 years ago

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 6 years ago by Don-vip (previous) (diff)

comment:11 by Don-vip, 6 years ago

Resolution: fixed
Status: newclosed

In 13671/josm:

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

comment:12 by Don-vip, 6 years ago

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 by Don-vip, 6 years ago

In 13678/josm:

see #16188 - rework new test

comment:14 by Don-vip, 6 years ago

Milestone: 18.04

comment:15 by Don-vip, 6 years ago

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

comment:16 by Don-vip, 6 years ago

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. 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.