Rendering specific access tags

[dch0ph]

Fixes #214

Changes proposed in this pull request:

The code proposed has been extensively discussed in #214, but in summary:

SQL functions introduced that interpret mode-specific access tags in addition to the overall access tag.

Tags considered are determined by a "primary mode" inferred from the highway types:
Vehicle road (primary mode: motorcar): motorcar > motor_vehicle > vehicle
cycleway: bicycle
bridleway: horse
footway, steps: foot
[Access tagging on track is unchanged, i.e. only determined by access]

In this initial PR, the interpretation of path is unchanged, i.e. path is considered to be a cycleway or bridleway if bicycle=designated or horse=designated respectively.

The access tags are reduced to a single int_access tag tagging the values no, restricted and yes (which is equivalent to NULL). restricted used to indicate an intermediate "some restrictions apply" for vehicles. The access marking used is the the current access=destination, but the name change reflects the fact that other values are included, e.g. delivery.

The int_access is generated, as normal practice for this style, on-the-fly. An option to use a generated column to pre-calculate these values is commented out. In practice, the overhead of combining the access tags is likely to be small given the vast amount of in-line SQL in the roads query. Note that some cruft in the railway side of the roads query has been removed.

Other global access tags, such as access=forestry, are also now interpreted (equivalent to no). access=agriculture;forestry is also accepted, although we don't typically interpret multiple-value tags.

The code does not require a database re-load, but does require additional functions to be installed in the Postgres database. These have been placed into a file functions.sql so that there is a place where functions for the style can be gathered. This does require an additional step in installing the style, and so installation instructions and the CI test have been adjusted. The Docker set-up has not been touched and will need fixing by somebody who understands/uses it.

Test rendering with links to the example places:

Destination marking
Residential highway tagged with motor_vehicle=destination.

Before

After

No marking
Before
access=yes, motor_vehicle=no, psv=yes

After

Honouring foot
North-south footway tagged with highway=footway, foot=private

Before

After

Honouring bicycle
highway=cycleway, bicycle=designated, access=no
Before

After

The last example needs discussion since it is a case where access=no has been added because the bridge is closed:

The logic of access tagging is that the general access=no is overridden by the specific bicycle=designated, and a router should allow bicycles across. So this usage is arguably tagging for the renderer: retaining perhaps a formal right of way (bicycle=designated) but indicating that the way is closed by exploiting Carto's simple approach to access tagging.

It is inevitable that a wider and more correct usage of the access tags will throw up such cases!

[imagico]

This is first look at the implementation, i have not actually tested it yet and this definitely requires thorough testing.

Overall, i think this is a coding wise a good design for the initial take on the subject. But this is my personal opinion based on me being fine with the concept of using SQL functions to modularize SQL functionality in the style. Others might see this differently.

If we use this approach what we should be aware of is that if we ever change the parameter lists or the names of the functions we will need to make sure our installation procedure drops the old functions - because that is not taken care of by the CREATE OR REPLACE. This should probably be mentioned in our developer documentation.

Will separately comment further on the handling of highway=track in #214.

[imagico]

You here distinguish between a two class and a three class access restriction model based on the primary mode - while we make this distinction based on road class. While this might not be significant in the exact design model you propose, it would be in many styling variation (like for example if you render highway=track with primary mode motorcar).

[dch0ph]

You here distinguish between a two class and a three class access restriction model based on the primary mode - while we make this distinction based on road class. While this might not be significant in the exact design model you propose, it would be in many styling variation (like for example if you render highway=track with primary mode motorcar).

Yes and no. I renamed the functions to carto_highway_primary_mode etc. and returned a "primary mode" for clarity, whereas it's really an "access type", so you could classify into motorcar_track if you wanted to have a 2 class access style for track. I could change it back to say access_mode. I was keen not to "reparse" all the way from highway to avoid repetition and improve efficiency.

[imagico]

Primary mode of use and road class are well defined concepts - you will need to explain what you exactly mean by "access type".

[dch0ph]

Fundamentally the "access category" is a data abstraction that keeps the code simple and flexible. A given way is classified into an access category, which mostly depends on the highway type, but for highway=path also depends on access tags. All the decisions can then be made on the "access category", e.g. this determines which mode-specific access tags are to be used, but it is also used to distinguish between 2-state and 3-states access renders. So it is a single value that encapsulate both the "primary mode" and how the different access values will be translated into yes|restricted|no. So in retrospect it was confusing to conflate the "access category" with "primary mode". Hopefully most of this is documented in the comments.

[imagico]

Fundamentally the "access category" is a data abstraction that keeps the code simple and flexible.

I think it is not a good idea to invent such concepts as parameters of implementing styling logic, especially if their meaning changes when changing the design logic.

The primary concern when implementing this should be that other style developers can easily understand and modify the tag interpretation logic. The secondary concern should be code performance since we are going to use it on the fly during rendering. Code complexity itself is lower priority.

As i said - we actually want to make this distinction based on road class, because it depends on the road class and its line signature if we can only show a two class access classification or if we can show three classes.

[dch0ph]

We can call it road_class rather than access type/category. That's all it is really. highway=cycleway and highway=path, bicycle=designated belong to the same "road class" because the access marking works in the same way.

[imagico]

Then i'd suggest to simply re-cast highway=path into highway=cycleway/bridleway/footway in SQL and work with that as parameter. You essentially already have the function for that (carto_path_primary_mode()). We currently do this re-casting in MSS code - but since you need it for the access restriction processing you can do it in SQL and then simplify the MSS code as a bonus.

You'd need an additional query nesting level for that or move the carto_highway_int_access() call to the outer query.

[dch0ph]

Then i'd suggest to simply re-cast highway=path into highway=cycleway/bridleway/footway in SQL

Yes, I was thinking along these lines - an effective int_highway. I am keen to collapse the different road types in one place, and will have to check that this works across the wider framework.

[imagico]

This is odd - if you have a distinct primary mode pedestrian (and it is unclear what this means relative to foot): why does this universally mean access=no. Note that while we currently do not render highway=pedestrian with access dashing, access restricted pedestrian roads of course do exist - like in private parks or in military facilities.

[dch0ph]

Again, perhaps renaming "access type" as "primary mode" has hindered rather than helped. The idea of a "pedestrian" access mode was an implicit motorcar/motor_vehicle=no. Putting this no into int_access simplifies later queries about whether to add colour for bus=yes etc.; only ways with int_access=restricted|no are considered.
The alternative is to set int_access=NULL and to treat highway=pedestrian as a special case [IF int_access=restricted|no OR highway=pedestrian THEN <consider bus=yes marking>]

[pnorman]

This seems flawed, as if you have, for example, access=foo motor_vehicle=bar hgv=baz then the access restriction for hgv is the value of the hgv tag, regardless of if we understand that value or not.

All we need to do is coalesce the default value and access tag hierarchy in the appropriate order and compare that with the values we're considering.

[imagico]

This seems flawed, as if you have, for example, access=foo motor_vehicle=bar hgv=baz then the access restriction for hgv is the value of the hgv tag, regardless of if we understand that value or not.

That depends on our strategy of dealing with unknown tags in general.

If we'd show hgv=* in that scenario then we would have to interpret hgv=baz as either of the three access classes we render. We could do that, but that would functionally introduce a kind of catch-all - interpreting all unknown values as synonyms for a specific known value. And i am not sure we'd want that.

The simplest alternative to introducing a catch-all would be to simply ignore unknown values (because we don't know what they mean - hence any interpretation we'd make would be wrong). That is what this PR currently does. I am not saying this is the only viable approach. But it is not clearly wrong either. In case of an unknown tag the reason might be a typo and the typo might be in the key or in the value. If it is in the key the approach of treating unknown tags as fully invalid and void would be the better.

The other alternative would be to introduce an additional visualization class meaning unknown - indicating that an access restriction is tagged for the primary mode of use but it is undefined - and therefore distinctly different from the case with no access restriction. This would go a bit in direction of #4723.

[pnorman]

We have to interpret unknown values regardless. If we have access=baz and it's unknown we still have to render the road one of the three ways.

The other alternative would be to introduce an additional visualization class meaning unknown

I don't think a 4th class is possible without becoming confusing.

[dch0ph]

This seems flawed, as if you have, for example, access=foo motor_vehicle=bar hgv=baz then the access restriction for hgv is the value of the hgv tag, regardless of if we understand that value or not.

I don't follow this specific point, since we don't consider the "access restriction for hgv".

But, I guess the general point is about
access=no, motor_vehicle=baz
since the two approaches will give different results.

Personally I feel that the outcome of the COALESCE approach will be more obvious to mappers (if motorcar is set, it's value will determine access marking, if not, motor_vehicle etc.)

We have to interpret unknown values regardless.

Good point. Currently there is an asymmetry between the mode-specific tags which are "vetted", and the overall access tag, which is not, primarily because we have to do something with unknown values.

[imagico]

We have to interpret unknown values regardless. If we have access=baz and it's unknown we still have to render the road one of the three ways.

Currently, if a road is tagged access=baz that tag is ignored, it is rendered as if the tag was not there.

So far the style universally ignores unknown tags (i.e. it treats them as if they were not there) - with the exception of the few catch-alls we have in the style (which we try to reduce - see #4568 and #4725).

So far all the catch-alls we deliberately have in the style are cases where an unknown value naturally is to be engrossed in a common design with other known values. Like unknown building=* being rendered like building=yes and unknown shop=* like known shop types without a distinct symbol.

This is different here, motor_vehicle=baz does not naturally fit into either of the three classes we render.

I don't think a 4th class is possible without becoming confusing.

I would agree if that 4th class was indeed another class of access restriction. But it would not be, it would be an error indicator. It would not transport information about the geographic reality, it would indicate that there is an error in the data that prevents us from reliably providing such information even though the other data clearly indicates that such information should be shown.

I had been sceptical about #4723 in general (and i still am) - but this specific constellation where there is no good solution other than explicitly showing cases where there is an error in the data is one where i would be in favor of an explicit error indicator.

[pnorman]

I don't think a 4th class is possible without becoming confusing.

I would agree if that 4th class was indeed another class of access restriction

If you don't like the word class, call them a distinct symbology. I don't think we can have four given the number of other things we render on roads.

Currently, if a road is tagged access=baz that tag is ignored, it is rendered as if the tag was not there.

So far the style universally ignores unknown tags (i.e. it treats them as if they were not there) - with the exception of the few catch-alls we have in the style (which we try to reduce - see #4568 and #4725).

A catch-all is when we're rendering all features regardless of what they are. That's what we've tried to reduce.

Deciding what to do with unknown access values is, on the other hand, something we must do if we render any. If for each road we render it with three different symbologies to represent different access restrictions, we must then have some way to decide which of the three to use given the access related tags. This includes when the access tags make no sense to us - unless we completely stop rendering the road, we still have to decide which of the three.

[imagico]

A catch-all is when we're rendering all features regardless of what they are. That's what we've tried to reduce.

#4568 and #4725 are cases of catch-alls in secondary tags where any and all values with a certain key other than a few explicitly handled otherwise have a distinct effect on rendering compared to no such tag being present.

Our goal to support mappers in consistent mapping practice IMO mandates us to reduce these because it is not only ill-defined primary (feature) tags being rendered without there being a consistent mapping practice in use that works against this goal, the same applies for secondary tags.

Deciding what to do with unknown access values is, on the other hand, something we must do if we render any.

As already said - our standing practice universally implemented in this style except for the few catch-alls we have is to ignore them.

If we are considering changing that we should look at it from the perspective of our goals, specifically here (a) the goal to support mappers in consistent mapping practice and (b) the goal for our design being intuitively understandable for mappers. Looking at the suggestions we have so far:

1. ignoring unknown access values (this is what this PR implements at the moment)

   • support of mappers in consistent mapping practice is good because only tags that we know are applied consistently go into the rendering - hence there is no counterproductive positive feedback on ill-defined tags.
   • intuitive readability is non-ideal because the tag interpretation breaks with the strict logic of access tag overrides in case of unknown values. It is, however, clearly not worse than the status quo where we treat all more specific access restrictions as unknown (i.e. completely ignore the hierarchical structure of the access tagging system).

2. interpreting unknown values universally as access=yes

   • support of mappers in consistent mapping practice is not ideal because the mapper get positive feedback in the form of a distinct effect on rendering results of undefined tags, potentially leading to proliferation of access values for only weakly restricted access (because those receive the semantically fitting positive feedback)
   • intuitive readability is good because it follows the consensus logic of acccess tagging strictly. It is, however, confusing in case of access values unknown to us that are either evidently non-sensical (why is this access=no road shown as open? Ah, because of the motorcar=who_cares), quite clearly severely restricted (from the list of somewhat common values on taginfo for example bus, emergency, restricted, discouraged) or explicitly unknown (like access=no + motorcar=unknown/motorcar=FIXME)

3. an explicit error indicator signature for unknown values

   • support of mappers in consistent mapping practice is good
   • intuitive readability is good if a suitable signature can be developed

Personally i am confident that an intuitively readable design could be found for the third option (under the paradigm of being an explicit error indicator and not simply a fourth class in addition to the three we have). But developing such is delicate design work and this PR was started under the premise of changing tag interpretation only and not the actual style design. So i am hesitant to suggest to @dch0ph to work on this.

Everyone should also keep in mind that, compared to other tags, completely undefined values with no consensus meaning are really rare in access tagging:

https://taginfo.openstreetmap.org/keys/motor_vehicle#values
https://taginfo.openstreetmap.org/keys/motorcar#values

For both of these the explicit unknown value is the most common one that we could not reasonably put into one of the three classes we have based on established mapping practice. These observations IMO also favors option one and three.

[dch0ph]

I would suggest the following for this PR:

Switch to the COALESECE approach so that unknown (=invalid + non-interpreted) tags are not ignored, but reach the "interpretation" level (the carto_int_access function).

Return int_acess=unknown (rather than NULL) for unknown tags. Alternatively UNKNOWN to indicate that this is a value generated by Carto rather than a user tag (access=unknown) [but see below].

It is then devolved to the MSS whether to show a render for unknown/UNKNOWN access. [In the current PR/MSS, unknown values would be ignored as only int_access=restricted or int_access=no are associated with access rendering.]

For both of these the explicit unknown value is the most common one that we could not reasonably put into one of the three classes we have based on established mapping practice.

My personal preference would be to strip out "access"=unknown in Lua as junk tagging, but the easy option would to return int_access=unknown for an unknown access tag. This would automatically include cases where unknown has been used.

[imagico]

I would strongly suggest first to develop consensus on the desired tag interpretation and then think about implementation details. Choosing a certain tag interpretation because it is more convenient to implement would be a bad idea.

In principle a mixture between approach 1 and 2 would be possible in the form that the explicit unknown value is ignored (i.e. access=no + motor_vehicle=unknown would yield no) while other values with undefined meaning are treated according to Paul's suggestion (i.e. something like access=no + motor_vehicle=restricted would yield yes).

[dch0ph]

In principle a mixture between approach 1 and 2 would be possible in the form that the explicit unknown value is ignored (i.e. access=no + motor_vehicle=unknown would yield no) while other values with undefined meaning are treated according to Paul's suggestion (i.e. something like access=no + motor_vehicle=restricted would yield yes).

That would work for me. Rather than
WHEN motorcar IN ('yes', 'designated', 'permissive', 'no', 'private', 'destination', 'customers', 'delivery', 'permit') THEN motorcar
we would have
WHEN motorcar <> 'unknown' THEN motorcar

This would be functionally equivalent to a simple COALESCE, but ignoring unknown on the mode specific tags.

There is the issue of what to do with access=unknown. It could yield int_access=unknown (along with other uninterpreted tags, such as motorcar=baz). Alternatively we could distinguish between these outcomes.

[pnorman]

interpreting unknown values universally as access=yes (Paul's suggestion)

This is not what I suggested. I made no suggestion as to what a way with unknown access for the primary method is.

There is some justification for treating unknown differently, as it's an explicit unknown as opposed to a value we don't understand. access=no motor_vehicle=unknown truly indicates an unknown value, as opposed to access=no motor_vehicle=foo indicates a value we don't understand. Falling back to the access=no is wrong in the second case because the access tag doesn't tell us anything about what the mapper specified for motor_vehicle access.

3. an explicit error indicator signature for unknown values

You're proposing not unknown values, but invalid ones. Do we do this elsewhere? In the past we have rejected becoming a QA layer as it is clearly incompatible with 3/4 purposes. What we do with the tracktype is different - we have a symbology we show for unknown values where no tracktype is supplied, but we're not aiming for a different symbology for values that we believe are errors.

• support of mappers in consistent mapping practice is good because only tags that we know are applied consistently go into the rendering - hence there is no counterproductive positive feedback on ill-defined tags.

There still is. If someone tags highway=residential access= they get the feedback that their tag worked, because it still displays as having access.

[imagico]

This is not what I suggested. I made no suggestion as to what a way with unknown access for the primary method is.

Ok, i removed the attribution to you for the approach 2 in my list. But since my aim here is to develop consensus on a concrete approach to access rendering this does not really bring us forward. It would be helpful if you'd indicate which concrete approach you'd favor and which approaches you would find acceptable. So far we have discussed approaches 1-3 from #4952 (comment) and the mixture between 1 and 2 with explicit unknown being ignored while other unknown/invalid values are treated according to approach 2. If you have a different concrete suggestion please say so.

There is some justification for treating unknown differently, as it's an explicit unknown as opposed to a value we don't understand.

This is exactly why i brought up the mixture approach as a fourth option.

You're proposing not unknown values, but invalid ones. Do we do this elsewhere?

No, as said, this would be a first in this style - but so is the combined interpretation of several tags in this form that leads to this problem in the first place.

The tracktype case is different because we have no implicit default there (while we have an implicit default of access=yes on roads). And we have no complex combined interpretation of several tags, hence ignoring invalid tracktype values and treating them as explicit unknown is functionally the same (in other words: approach 1 and 3 would be functionally identical).

• support of mappers in consistent mapping practice is good because only tags that we know are applied consistently go into the rendering - hence there is no counterproductive positive feedback on ill-defined tags.

There still is. If someone tags highway=residential access= they get the feedback that their tag worked, because it still displays as having access.

No, because adding (for example) access=yes_except_for_tanks tag does not change the map rendering compared to not having the access=yes_except_for_tanks tag - hence no positive feedback. While in approach 2 tagging access=no + motor_vehicle=yes_except_for_tanks would change the rendering compared to just access=no. Hence the mapper adding motor_vehicle=yes_except_for_tanks would get positive feedback on their change.

[imagico]

@pnorman - reacting with a 👎 to my comment but not explaining what you disagree with is decidedly non-helpful. If there is anything unclear about the list of options i sketched please ask. If i misunderstood your comments and you think you have made a concrete suggestion different from the ones i listed please explain. If you disagree with my approach to developing consensus on this matter overall please take the initiative and explain what concrete solutions you deem viable.

[dch0ph]

To simplify further discussion (but not to prejudge consensus on tag interpretation), I've committed a bunch of changes that emerged from comments to date.

1. Clarified the purpose of the functions by returning an effective int_highway, a consolidated highway indicator which determines how access will be handled, e.g. all road types consolidated under road.

2. Changed logic of "coalescing" mode-specific tags so that only motorcar=unknown etc. are ignored. carto_int_access determines how the final coalesced tag is to be interpreted, with unknown values (including from access=unknown) returning 'unknown', potentially allowing this error result to be rendered.

3. The "promotion" of highway=path depending on bicycle and horse is now handled in SQL by returning e.g. feature=highway_cycleway for highway=path, bicycle=designated. This puts all the access handling in SQL, and the horse, bicycle (and unused foot) tags don't need to be passed into MSS. highway=path is still distinguished from highway=footway in SQL, even though the rendering in MSS is the same. The simplification of the MSS removes 2k lines of XML (4%).

I have checked that update code works as expected for the previous test cases, but have not exhaustively considered the various edge cases discussed. Ideally this would involve a "test sheet" or a shareable demo server.

I noticed that the roads query seems to include some cruft on the railway branch, e.g. evaluation of an int_surface! This could be a separate PR alongside other follow-up PRs.

[imagico]

From a quick look this seems like a structure that can be adjusted to any of the options in tag interpretation discussed so far so works well in terms of our goal of adaptability. Same applies to adjusting the tag re-casting of highway=path into other types (which can be done by modifying the carto_path_type() function).

You have not yet made any changed to the pedestrian logic - you still universally have pedestrian roads as no. Considering we don't show access restrictions on pedestrian roads at the moment this might seem a reasonable shortcut - I would still suggest treating them like footways in terms of primary mode and like roads in terms of having a separate restricted class. This would allow customization (or reuse of the SQL code in other styles) where restrictions on pedestrian roads are shown.

[dch0ph]

Thank for you the encouraging comments.

I'll admit that limited thought went into the pedestrian case; the automatic no access marking aimed at providing a "base" marking which would be coloured for permitted traffic, e.g. psv=yes.

I'm not convinced that a primary foot mode works here. They are basically vehicle roads with signage to restrict vehicles in much the same way as ordinary roads. I don't see how you would interpret/use the foot tag.

I would counter with something like:

  CASE
  	WHEN motorcar <> 'unknown' THEN motorcar
  	WHEN motor_vehicle <> 'unknown' THEN motor_vehicle
  	WHEN vehicle <> 'unknown' THEN vehicle
  	ELSE COALESCE("access", 'no') END

i.e. use the vehicle tags as for roads, e.g. motorcar=no, motorcar=destination, but default to no if there is no access tagging.

It might be interesting (separately) to develop a marking for restricted access on pedestrian ways.

If the outcome were int_access=yes (e.g. access=yes or motorcar=yes) + psv=yes, then there would be no psv allowed marking. But this is probably the Right Thing, since this would be fairly nonsense tagging for a pedestrian way.

[imagico]

I'm not convinced that a primary foot mode works here. They are basically vehicle roads with signage to restrict vehicles in much the same way as ordinary roads. I don't see how you would interpret/use the foot tag.

No, that is a wrong understanding of highway=pedestrian i think. These are footways that are wide enough to be used with a normal car but where cars are not allowed (and which therefore have an implicit motor_vehicle=no - or vehicle=no - depending on local conventions). Signage is not a requirement for highway=pedestrian - many are simply physically impossible to be used with a car.

foot=yes would be redundant on a highway=pedestrian and foot=no would be troll tagging. But many other values can be sensible in certain situations. foot=private on highway=pedestrian in a private park i already mentioned. Of course you could equally use access=private in such cases (and this is more common in practical use) - but foot=private is equally valid. foot=designated is also meaningful tagging (and should yield yes when used in combination with an explicit access=no)

[dch0ph]

No, that is a wrong understanding of highway=pedestrian i think. These are footways that are wide enough to be used with a normal car but where cars are not allowed (and which therefore have an implicit motor_vehicle=no - or vehicle=no - depending on local conventions). Signage is not a requirement for highway=pedestrian - many are simply physically impossible to be used with a car.

Agree that signage isn't necessary. I suspect that usage varies a lot between mappers (not least based on the wiki examples). But personally I only use highway=pedestrian where vehicle usage is possible (i.e. part of the road network, not separated by kerbs), e.g. a shopping centre/mall route would be highway=footway, but clearly some mappers would use highway=pedestrian for this.

From a quick scan on taginfo (743k uses overall) and Overpass:

Most access tagging is for bicycle (e.g. 62k bicycle=yes). Separate (but interesting) issue.

foot=yes has 24k. This is mostly fairly redundant highway=pedestrian, foot=yes.

access=private has 7k uses. This seems mostly on pedestrian areas (area=yes) on private grounds (which would be off topic).

There's a bunch of vehicle-related tags: vehicle=no (5k), motor_vehicle=private (4k), motor_vehicle=destination (4k).

But also motor_vehicle=yes (!). These mostly seem to correspond to "shared use" ways in public areas, where perhaps living_street is more appropriate.

access=no (5k): These mostly seem to be poor tagging, where a mode specific tag should be used (highway=pedestrian, access=no). But also includes examples on military land (although obvious from military hatching pattern).

foot=private (4k): on private land.

So there arguments for both routes:

Treating as a footway, and considering foot and access, in order to distinguish private ways. Personally I feel this is just less "interesting", and would need a careful choice of rendering that was different from vehicle access.

OR

Focussing on vehicle access. That would immediately allow things like:

for no general vehicle access, but psv allowed.

It would be useful to distinguish between motor_vehicle=no and motor_vehicle=destination. You could then add a render for "vehicle=restricted" (e.g. invert the current destination marking to white circles?).

In principle you could support both, with an additional int_access result, e.g. int_access=footno. Given the multiple uses of highway=pedestrian this might be a reasonable (if inelegant) option.

[imagico]

I think this is going too far outside the topic of this PR and #214. As indicated at the moment we are not rendering access restrictions on highway=pedestrian. We are discussing what would be a sensible default for the tag interpretation functions we introduce here for this road type based on the paradigm of visualizing restrictions for the primary mode of transport. I don't see a basis for considering the primary mode of transport for highway=pedestrian to be something other than foot as far as these functions are concerns.

As far as a hypothetical rendering of exceptions from implicit access restrictions (like on psv=yes on highway=pedestrian) is concerned - i mentioned the possibilities to do that here. The idea you sketch would mean stopping to render road types with implicit access restrictions as a distinct road classes and not rendering access restrictions based on primary mode of transport but based on a universally assumed motorcar default. That would mean all road types with implicit restrictions for motorcars would universally get a restriction dashing - which, as mentioned in the linked to blog post, would make larger pedestrian areas in urban centers very noisy (because they are all disallowed for motorcar).

But again - this is a discussion we should not have here. If and when the topic practically comes up the proper way to decide this would be to test how this would look like in practical rendering on a larger scale.

[dch0ph]

OK. In the interests of keeping things moving I've adjusted the access interpretation of highway=pedestrian:

WHEN 'pedestrian' THEN carto_int_access('pedestrian', CASE WHEN foot <> 'unknown' THEN foot ELSE "access" END)

i.e. using foot (as footway), but keeping the 3-state "rendering" associated with roads. Hence foot=delivery will result in int_access=restricted, although this will have no effect since there is no access marking on highway=pedestrian

[imagico]

The pedestrian road part looks good to me now.

Overall this seems essentially untested so far: functions.sql does not run:

psql:functions.sql:25: ERROR:  syntax error at or near "END"
LINE 22: END
         ^
psql:functions.sql:41: ERROR:  syntax error at or near "END"
LINE 14: END
         ^
psql:functions.sql:59: ERROR:  syntax error at or near "END"
LINE 16: END
         ^
psql:functions.sql:83: ERROR:  syntax error at or near "END"
LINE 22: END
         ^

Anyone who wants to see this move forward can help by testing and reporting any issues. Useful would in particular an analysis if the introduction of the functions leads to substantial performance decrease. This can be looked at through pg_stat_statements.

[dch0ph]

That's strange. I've been running the code on my own setup for some while, but the bridleway typo (now fixed) had slipped through.

The function load works in the github CI:

This step does need to be 100% reliable. I hope it's not a postgres version issue.

[pnorman]

@dch0ph do you have the ability to mark conversations as resolved? If so, can you indicate they are for the old comments, and if not, can you leave a comment saying it's fixed.

I consider it more important to have our code easy to read and understand than for it to be adaptable to different styles. I don't believe we should make the code more complex in order to make it easier for other people to potentially make changes.

I might rank this differently if it were a different section of the code, but our roads code is already complex to the point of being incomprehensible and the largest barrier to customization is understanding it.

My strong preference would be to release 5.9.0, merge the flex PR, make the necessary column changes and normalization at import time, and then merge the style changes for being part of 6.0.0

[imagico]

Having done a lot of work based on the road layers during the past years i can say that my experience here is very different. The main barrier to do any meaningful work in the road layers beyond fiddling with minor styling adjustments is the lack of structure, modularization and functional separation in the code, the extensive duplication of SQL in the road layers and the endless spaghetti MSS with interleaved use in different layers. What this PR is doing in that regard is trying - in a very small way - to not add to this mess but to start using new means (in this case SQL functions) to take a first step to break out this dead end.

Anyway - even if we agree to disagree on that: You have not presented any alternative approach for @dch0ph to pursue in solving #214 compared to the approach they took. In other words: Your suggestion seems to be to forgo addressing #214. I don't consider that acceptable.

My strong preference would be to release 5.9.0, merge the flex PR, make the necessary column changes and normalization at import time, and then merge the style changes for being part of 6.0.0

Let's discuss release planning in #4981. And let's discuss the question of if to do styling specific tag interpretation at import time in a separate issue.

[dch0ph]

@dch0ph do you have the ability to mark conversations as resolved? If so, can you indicate they are for the old comments, and if not, can you leave a comment saying it's fixed.

@pnorman I've resolved the queries raised in your last review where the query is clearly closed / resolved / superceded. There look to be two minor unresolved points (minor in the sense that they do not affect the outcomes and are more about the interpretative framework):

1. Whether to return, say, unrecognised when the access tags could not be sensibly resolved (rather than overloading the meaning of unknown).
2. The handling of the outcome of carto_path_type. I am happy with the current simplified code.

[pnorman]

You have not presented any alternative approach for @dch0ph to pursue in solving #214 compared to the approach they took.

I suggested resolving it by moving the SQL logic to import time.

In other words: Your suggestion seems to be to forgo addressing #214. I don't consider that acceptable.

I never said that.

[pnorman]

• Whether to return, say, unrecognised when the access tags could not be sensibly resolved (rather than overloading the meaning of unknown).

We should avoid the overload as it causes confusion.

2. The handling of the outcome of carto_path_type. I am happy with the current simplified code.

I'm okay with it too.

The major issue remaining is where the processing should be done, and the maintainers will need to come to a consensus on that.

[imagico]

I suggested resolving it by moving the SQL logic to import time.

I have said multiple times already - this is a suggestion that requires looking at a much larger context. We should discuss it separately from this PR. If you want to shelve addressing #214 until we have had that discussion and reached consensus that is fine - but then please open an issue for that and present arguments why you think this would be beneficial for this project.

[dch0ph]

We should avoid the overload as it causes confusion.

Addressed.

[imagico]

For clarity my updated review of this:

• I support differentiating between the explicitly tagged unknown and the effective classification as unrecognized.
• I maintain my previous assessment from Rendering specific access tags #4952 (comment) that slightly shortening the highway=path logic by depending on the assumption that carto_path_type() returns cycleway only if bicycle is set and bridleway only if horse is set is not a good idea. If no agreement can be found on the matter i am not going to insist on this but i would then insist on adding a warning comment explaining this dependency and i would also suggest to keep the more general variant commented out so style developers who want to adjust the logic can easily move to that.

[dch0ph]

• but i would then insist on adding a warning comment explaining this dependency and i would also suggest to keep the more general variant commented out so style developers who want to adjust the logic can easily move to that.

I have added this. I prefer the simpler code, where "decisions" around, say, highway=bridleway only involve the value of horse and not any other access subtags.

[pnorman]

I have said multiple times already - this is a suggestion that requires looking at a much larger context. We should discuss it separately from this PR. If you want to shelve addressing #214 until we have had that discussion and reached consensus that is fine - but then please open an issue for that and present arguments why you think this would be beneficial for this project.

I cannot agree that a discussion of putting logic in functions is out of scope on a PR that is the first to introduce logic in functions.

[imagico]

I have added this. I prefer the simpler code, where "decisions" around, say, highway=bridleway only involve the value of horse and not any other access subtags.

This is not really relevant here at the moment since we are not discussing a change in the logic of how to interpret highway=path. But i would still like to understand this. Are you saying you would like to adjust our tag interpretation logic based on what can be implemented in the framework chosen in the most compact form?

Because i am usually taking the opposite approach. I think about what is map design wise the desirable logic of displaying things and then i think about the most elegant way to implement that is the framework we have. Adjusting map design to the technical constraints is of course necessary at times - but this is typically only of concern with geometric aspects and spatial relationships, not with tag interpretation.

@pnorman:

I cannot agree that a discussion of putting logic in functions is out of scope on a PR that is the first to introduce logic in functions.

As before you seem to disagree with something i have not said. Anyway - it is fair enough that if i ask you to make your case of doing style specific tag interpretation in preprocessing in a separate issue i should likewise discuss the idea of introducing SQL functions in a separate issue - which i have done in #5004.

[dch0ph]

Are you saying you would like to adjust our tag interpretation logic based on what can be implemented in the framework chosen in the most compact form?

No. I'm just arguing to keep the code in functions as clear as possible. If somebody does want to change the logic, they first need to understand what the code is doing. The natural reaction to looking at
carto_int_access(COALESCE(NULLIF(bicycle, 'unknown'), "access"), FALSE)
after the carto_path_type code is "why is this so complex when you know bicycle is not unknown?". This was pnorman's reaction. People are reluctant to change code when they don't understand its logic.

In the end, the "more general formulation" (which I initially used from a simple cut and paste) is not adding anything useful. It only has an effect if somebody has created a perverse carto_path_type that promotes path to cycleway despite bicycle either not being set or when bicycle=unknown.

[imagico]

Thanks. I think i understand the consideration behind that approach better now. It is also my experience that people have very different approaches to understanding logic and its implementation in code. It is best not to infer from one's own approach to understanding on how others will see this.

Anyway, with the warning comment added i think this is ok.

It only has an effect if somebody has created a perverse carto_path_type that promotes path to cycleway despite bicycle either not being set or when bicycle=unknown.

It is not a perverse way of casting highway=path into our three classes, it is a perfectly valid variant of doing it. I have not thought about it very much yet but at the moment i would probably lean towards showing a highway=path with foot=no and horse=no but no bicycle=* as cycleway.

[imagico]

Doing final review i noticed that this currently breaks bridge/tunnel rendering on highway=path - you either need to modify carto_path_type() to not return path or re-add highway_path selectors in MSS code at some places.

[imagico]

This needs to be footway rather than path if you want to eliminate highway_path from MSS altogether.

[dch0ph]

The conservative option is to keep the option of rendering highway=path differently from highway=footway (there already seems to be some difference in that footway can have an area render, but not path).

[imagico]

Ok, this seems to work fine now also with tunnels and bridges. I am fine with leaving highway_path as a distinct feature class for those features that are not re-cast to cycleway/bridleway. As you point out this is still a difference w.r.t. polygon rendering.

Regarding foot/bicycle/horse being distinct columns while the other mode specific access tags being in hstore - that will need to be considered in the next database reload. But of course backwards compatibility of the database structure is a factor to considered here. Historically, we have rarely dropped columns. When you want to run an older version of OSM-Carto on the current database structure that requires relatively few changes. That consideration should not prevent us from consolidating the database layout, but we should be mindful of these things and make considerate decisions with an overall strategic perspective.

The other big thing we have discussed in the exact tag interpretation logic. I am fine with keeping this as it is if this is the smallest common denominator we can agree on.

To summarize: Currently this PR classifies accessvalues (i.e. values of tags indicating access - either generic, access=accessvalue or transport mode specific, like motorcar=accessvalue) into a total of six classes:

1. recognized yes values ('yes', 'designated', 'permissive')
2. recognized no values ('no', 'permit', 'private', 'agricultural', 'forestry', 'agricultural;forestry')
3. recognized restricted values ('destination', 'delivery', 'customers')
4. explicit unknown - these are ignored (treated as if not tagged)
5. NULL (i.e. not tagged) - these are ignored
6. any other values (unrecognized)

After this initial classification the different access tags are then conflated into one total classification based on the principle:

For the primary mode of transport we selected for the road class in question the more specific tags (like motorcar=*) override the more general ones (like vehicle=* and ultimately access=*), unless it is in an ignored class (4 or 5) according to the list above.

The resulting total classification is then cast into the visual classes we have:

• yes is shown with the plain road signature
• no is shown with the gray centerline dashing (for the wide roads) or with a gray/desaturated color (for the narrow roads)
• restricted is shown with the gray centerline dotting (for the wide roads) or conflated with yes (for the narrow roads)
• unknown/NULL is conflated with yes
• unrecognized is conflated with yes

I don't think this is a good way to deal with the unrecognized values based on the goals of our project and i have also presented a concrete alternative (approach 3+ as described above). I don't want to push for it here - after all it affects only the very small percentage of access tags that use a value other than those we explicitly interpret. But i still want to - for the record - point out that an alternative has been suggested and is viable (both technically and design wise).

The PR, overall, seems to work fine now in all constellations i have tested and unless something new comes up suggesting otherwise (further testing by others would be appreciated) i intend to merge this as per what was said in #5004 (comment).

partly resolved, partly superseded by discussion on #5004

[imagico]

Merged now, thanks @dch0ph for the patience in working on this.