Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rethink handling of fenced=yes #514

Open
Hufkratzer opened this issue Jun 25, 2022 · 12 comments
Open

Rethink handling of fenced=yes #514

Hufkratzer opened this issue Jun 25, 2022 · 12 comments
Labels
bug Something isn't working deprecating

Comments

@Hufkratzer
Copy link
Contributor

Hufkratzer commented Jun 25, 2022

Current behaviour

iD seems to produce warnigs for all objects with fenced=yes that look like
fencedmeadow
because of

"old": {"fenced": "yes"},
"replace": {"barrier": "fence"}

If the user executes the option to "upgrade the tags". iD replaces fenced=yes with barrier=fence on the same object.

Why this is not a good idea

fenced=yes is a tag mainly for area features, most of the objects tagged with it do also have an area tag, mostly landuse=* or leisure=*. In contrast, barrier=fence is a tag for a linear feature (a fence). For these objects, replacing fenced=yes with barrier=fence produces objects that are tagged with an area and a linear feature tag mixed together. This is generally discouraged, explained in these discussions:

Possible solutions

  • Replace the current option to "upgrade the tags" with another one that produces a separate way with barrier=fence (and its attributes, if present). There is already a similar option for nodes that are part of a way but shouldn't:
    pitch
    Also objects that already have got these mixed area and linear tags should be handled.
  • Don't do anything in case of fenced=yes and ignore its deprecation (until some better solution than the current one is available).
  • Question whether fenced=yes should be deprecated at all.
@tordans
Copy link
Collaborator

tordans commented Jun 25, 2022

I agree, that most of the time one should map the fence separate from the area (in fact in my experience very few areas are actually fenced all the way around).

However, I don't see an issue with having an "area-feature" + barrier=fence, if mapping the fence separately is for some reason not feasible (at this point in time).

However(2), I do see an issue with having two tags that describe the same thing. So I agree with the deprecation and upgrade-nudge.

I agree, that having a "magic feature" that migrates the old tagging to a "two separate but connected geometries"-solution, would be an even better "deprecation" / upgrade path. But that is a complex feature to build…

@1ec5
Copy link
Contributor

1ec5 commented Jun 25, 2022

The possibility for a single feature to be an area and a linear feature at the same time is a gotcha that many data consumers have run into and needed to work around, because it isn’t interoperable with other GIS formats and schemas. The workaround is usually to synthesize a coincident feature (representing just the barrier=fence in this case). OSM’s data model doesn’t prevent someone from dual-tagging a feature in this manner, but iD shouldn’t encourage it. I think it would make a lot more sense to separate the two features, even if that means an area would be glued to a line all the way around: openstreetmap/iD#8263 (comment).

I agree, that having a "magic feature" that migrates the old tagging to a "two separate but connected geometries"-solution, would be an even better "deprecation" / upgrade path. But that is a complex feature to build…

Validator rules and fixes are implemented downstream in the iD codebase. If the “crossing ways” rule’s “Connect these ways” fix is any guide, a similar “Redraw the fence as a line around this area” fix should be relatively straightforward. The “outdated tags” rule is powered by the data in this repository, but any fix more complex than a simple tag replacement should be tracked in the openstreetmap/iD repository.

@Hufkratzer
Copy link
Contributor Author

However(2), I do see an issue with having two tags that describe the same thing.

fenced=yes and barrier=fenced don't "describe the same thing". just as covered=yes and building=* don't.

  • fenced=yes describes an object that is fenced (attribute) by another object (a fence) that does not necessarily need to be mapped.
  • barrier=fence describes a fence object (feature), and what it fences in (e.g. a pitch) does not necessarily need to be mapped.

Other comparisns see here

@tyrasd tyrasd added bug Something isn't working deprecating labels Jul 1, 2022
@tyrasd
Copy link
Member

tyrasd commented Jul 1, 2022

Question whether fenced=yes should be deprecated at all.

FWIW, fenced has been documented as a deprecated tag since 2016 on the wiki: https://wiki.openstreetmap.org/wiki/Key:fenced. If anything, this discussion should be made outside of this repository.


I agree that the current deprecation rule doesn't really do the right thing for polygonal features. A fix would need to:

  1. remove the deprecation rule from this repository and
  2. implement a dedicated validation rule on the iD repository for it which adds a separate barrier=fence way around the area

@matkoniecz
Copy link
Contributor

fenced has been documented as a deprecated tag since 2016 on the wiki: https://wiki.openstreetmap.org/wiki/Key:fenced

It is not changing that mass replace with barrier=fence is making situation even worse.

Yes, wiki has/had (I edited it)

This feature has been labeled as deprecated. The recommended replacement is: barrier=fence.

at https://wiki.openstreetmap.org/wiki/Key:fenced but many OSM Wiki pages claim silly things and should not be followed blindly.

@Hufkratzer
Copy link
Contributor Author

Question whether fenced=yes should be deprecated at all.

FWIW, fenced has been documented as a deprecated tag since 2016 on the wiki: https://wiki.openstreetmap.org/wiki/Key:fenced. If anything, this discussion should be made outside of this repository.

Do you need a decision about that outside of this repository before you change the handling in iD or do you want to do what you already have proposed (replace fenced=yes by barrier=fence on a new object).

@tyrasd
Copy link
Member

tyrasd commented Jul 15, 2022

For now, I go with how it is currently documented: replacing fenced=yes by a new barrier=fence object. But I would also be ok to remove that "tag upgrade rule" at some point in the future should the community consensus change such that fenced=yes is ok to be mapped (analogously to coveverd=yes for example).

@matkoniecz
Copy link
Contributor

The problem is that fenced=yes may be also applied where object is almost, but not entirely fenced or parts of perimeter is blocked by other features, such as buildings.

In such cases robotic adding barrier=fence around entire area does not help.

@Hufkratzer
Copy link
Contributor Author

Hufkratzer commented Aug 12, 2023

FWIW, fenced has been documented as a deprecated tag since 2016 on the wiki: https://wiki.openstreetmap.org/wiki/Key:fenced. If anything, this discussion should be made outside of this repository

The status of fenced=yes was changed to in use on June 14, 2023.

Maybe the deprecation rule in iD can be removed now.

@tordans
Copy link
Collaborator

tordans commented Aug 13, 2023

When we remove the deprecation rule, do we want to also add a moreFields field for it? Otherwise editors will more or less ignore the tag. — Personally I am fine with it, since I (a) don't understand why we need it at all; (b) I am not aware of any data consumers that use it (one could always check for the fence-object itself); (c) I would not know to which id-tagging-schema presets to add it to and which to leave out (could be researched…).

@Hufkratzer
Copy link
Contributor Author

Maybe the deprecation rule in iD can be removed now.

I just realized that this was already done long ago (910ef16). I had overlooked it because this issue remained open and the discussion continued. Sorry for the unnecessary noise.

When we remove the deprecation rule, do we want to also add a moreFields field for it?

Interesting idea, but I don't know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working deprecating
Projects
None yet
Development

No branches or pull requests

5 participants