Remove name from vacant shop when it's no longer vacant.

[TurnrDev]

Vacant shops sometimes have the name key still set to help make it identifiable when the house number can't be determined in person. (As shops often don't care about removing signage when they close for good)

When answering this quest on such an element, the name key is often retained and the data is left bad.

I'll provide an example:

A bar near me closed down ("The Black Cat") but left their signage up. I left the name tag on the element when I set "shop=vacant" as it's in a dense part of town where it'll be hard to know what element StreetComplete might be asking for otherwise. A new establishment opened in its place but StreetComplete never asked me for the name as the old bars name wasn't removed from the "shop=vacant" element.

Whether or not shop=vacant or disused:shop=* elements should have names, 30% of them do, so it's worth covering the edge case :)

[mnalis]

I agree that the old names (if left standing) are still useful orientation points, even if the shop/amenity is vacant.

So probably the most useful would be to leave the name when shop is marked vacant (as you did), but when user in SC answers that the shop is no longer vacant; then remove the name tags. (and the user can enter new name in subsequent quest, after the shop is no longer marked as vacant/disused)

[TurnrDev]

when user in SC answers that the shop is no longer vacant; then remove the name tags. (and the user can enter new name in subsequent quest, after the shop is no longer marked as vacant/disused)

Yeah, that's what this does :) Wasn't sure if my explanation was clear enough

[westnordost]

Also apart from the use case you described, removing name makes sense if it is still set. I wonder if other tags should be removed as well?

Also, did you abandon your earlier PRs?

[matkoniecz]

At least opening hours tag, if not removed already

[TurnrDev]

Also, did you abandon your earlier PRs?

They've kind of been put aside indefinitely until I finish some paid work. I do plan to finish those PRs soon, just can't say when. Unless you want to give me a deadline to panic me into doing them 😅

[westnordost]

No, just asking because if they were abandoned, I could mark that somehow that it is up for the taking, i.e. could be finished by someone else who is willing to.

[TurnrDev]

Oh, absolutely, if someone wants to finish them, they can :)

[TurnrDev]

I've just added a few more keys to be removed which would depend on the previous establishment

[Cj-Malone]

fhrs:id and brand:wikipedia should also get removed.

[westnordost]

Hmm... maybe turn that around... what should not be removed. IIRC this is how it is done when the user selects "this shop is vacant now"

[TurnrDev]

Hmm... maybe turn that around... what should not be removed. IIRC this is how it is done when the user selects "this shop is vacant now"

• any addr:* tags
• building and any tags which describe attributes of building
• ref:GB:uprn
• level
  I guess?

[Cj-Malone]

Address data should defiantly not be removed, there is a lot of effort going into getting open addresses in the UK. However I'm pretty sure there is an ancient OSM rule along the lines of "If you don't understand a tag, leave it.", so I'd still argue for an explicit list of removable tags, and leave everything else.

[goldfndr]

Perhaps move the name to old_name instead of complete removal?

[westnordost]

there is this constant:

StreetComplete/app/src/main/java/de/westnordost/streetcomplete/data/quest/QuestController.kt

Lines 245 to 257 in dbfe44e

	private val KEYS_THAT_SHOULD_NOT_BE_REMOVED_WHEN_SHOP_IS_REPLACED = listOf(
	"landuse", "historic",
	// building/simple 3d building mapping
	"building", "man_made", "building:.*", "roof:.*",
	// any address
	"addr:.*",
	// shop can at the same time be an outline in indoor mapping
	"level", "level:ref", "indoor", "room",
	// geometry
	"layer", "ele", "height", "area", "is_in",
	// notes and fixmes
	"FIXME", "fixme", "note"
	).map { it.toRegex() }

This could be reused (=moved to some file like OsmTaggings.kt) right?

[westnordost]

Are you planning to finish this PR?

[TurnrDev]

Yes, today actually 😊

[TurnrDev]

I'm not sure we should be removing tags were unsure of

[TurnrDev]

Whilst I'm on the mindset of "if you don't know what a tag is, don't touch it", I added some important tags to the don't delete constant you mentioned above.

[TurnrDev]

@westnordost Since I can't access current element tags from CheckShopType.applyAnswerTo() I can't build a sting map of changes to remove all except the tags we want to keep. Gonna have to stick to only removing what we intend to remove.

[TurnrDev]

I'd say this PR is ready for a review :)

[westnordost]

@westnordost Since I can't access current element tags from CheckShopType.applyAnswerTo() I can't build a sting map of changes to remove all except the tags we want to keep. Gonna have to stick to only removing what we intend to remove.

But you can. changes.getPreviousEntries() returns all the tags currently tagged on the element.

[matkoniecz]

You can also take a random shop, disable automatic uploads and apply edit to it - and see logs whether SC takes correct actions.

[TurnrDev]

You can also take a random shop, disable automatic uploads and apply edit to it - and see logs whether SC takes correct actions.

Just saw this, didn't think of logs but this shows they applied the right tag changes (removing name)

Just tested now

[westnordost]

LGTM

[Woazboat]

As was already brought up by others, I believe a blanket-removal of all tags aside from a few that are on a list is a bad idea. Such a list will always be incomplete and also doesn't consider local practices. The reverse approach of only removing tags that are known to be associated with a shop and are likely to change would be a much better idea and is unlikely to clobber valid data like a blanket removal would. Also, what happens if a new tag or tagging scheme is invented? Will this list constantly be updated? Who decides which tag is worthy to be added to this list or not? What about users with old versions of StreetComplete? What if StreetComplete development stops some day?

I also support moving the name to old_name instead of completely removing it.

[TurnrDev]

I agree with you @Woazboat

[mnalis]

Great points @Woazboat. To me it also seems MUCH better to only remove the explicitly named tags, instead of removing everything except explicitely named tags. For example, source* tags (they are still valid, and a lot are of before-changset-source tags). wikipedia/wikidata might be describing the building in which a shop is located too. I've seen ref:ruian is being added (which I've never heard of before, how many other such tags exist, or will exists in the future)? There are several thousand barrier tags on those shops too, according to taginfo. What about ncat (whatever that is, can't even read that alphabet)? utahagrc:parcelid and WroclawGIS:building:ID also seem to be related to location/building, not shop. Those are just random missed tags, I'm sure there's plenty more.

I guess one might wade through several thousand of more popular entries at taginfo to check what more should be left alone. Problem is that is not only error prone (and hard to decide sometimes), but also that new tags pop up in use all the time (some documented, some not).

To conclude, what is boils down to is: when (not if) the error does happen and SC makes a wrong choice, I'd much rather we err on the side of "we sometimes leave some potentially stale information we didn't think of" than "we sometimes destroy useful and valid information". I still think it is valid reasoning, even when it is more work - intentionally destroying potentially useful information makes me cringe, even if the amount destroyed would likely be relatively/quite small.

Or alternatively we can make a hybrid (but that sounds like more work for less gain):

• remove explicitly named tags that we always want to remove
• keep explicitly named tags that we always want to keep
• rename name to old_name
• rename all remaining tags by prefixing them with disused:

[matkoniecz]

rename name to old_name

This is a bad idea, only very rarely old_name should be kept and deleting it is not some bad edit. And in many cases it would be harmful/pointless.

In manual editing I basically never keep it.

[mnalis]

@matkoniecz what about the rest of my suggestion, which was the main point? Do you have an opinion yet if you would rather prefer potential data loss, or potentially obsolete data?

As for the old_name digression, I personally very often retain old_name (especially for street, cafe, and shop) when editing manually. About pointless of it: If the data consumer does not care for it, it does not have to use it, eh? I do not however see how could recording previous name be harmful, though (it is not like it's not documented what it describes, or that people try to force it inside regular name tag or something where it could cause confusion / harm)?

I personally do find old_name useful, mostly dealing with navigation and orientation:

• for streets/squares (as part of address, vital for navigation, but often changed by politicians, and people also often know only some of the names: eg. https://www.openstreetmap.org/way/591197186)
• for cafes/pubs/clubs: also navigation, often you're supposed to meet near some legendary (in local folk culture) cafe, which has since (sometimes many years since) changed owners and name, but is still referred by its previous name. Like https://www.openstreetmap.org/node/1765348807 or old pictures/signs still remain https://www.openstreetmap.org/node/1377483198
• for shops: navigation/disambiguation, for example we used to have two separate convenience stores with different names in the neighborhood, but one got bought by other, and now they are both having same name; so the one that got bought out is still referred to by its previous name. https://overpass-turbo.eu/s/19MZ (which would be highly confusing if not for old_name).

Sure, sometimes (if the previous node was short lived, rarely used, of no special interest even to locals, and no mentions of it remain), value of old_name is very low indeed (say, on the par with usefulness of roof_type), but in non-negligible number of cases it is actually quite useful. It would be bad to indiscriminately destroy both (and asking the user if the old name should be preserved for each such element is an obvious no-go for StreetComplete), IMHO.

[kjonosm]

There is also disused:name=* with approx. 20,000 uses. See also https://taginfo.openstreetmap.org/keys/disused:name

[matkoniecz]

I do not however see how could recording previous name be harmful, though

OSM is not for historic data, and presence of old_name and other similar encourages people to map other historic data. We have already problem with railways where people sometimes try to map features completely and utterly gone without any trace whatsoever in any form. Up to and including former railway going through place that is now 30m deep pit of surface mine.

So I am quite allergic to mapping historic no longer existing objects or suggesting that it would be OK (with exceptions for temporary notes protecting against invalid armchair edits)

for streets/squares

Not relevant at all for handling vacant shops (also, unlike shops such old names are typically in actual use)

for cafes/pubs/clubs: also navigation, often you're supposed to meet near some legendary (in local folk culture) cafe (...) for shops: navigation/disambiguation

That is extremely, ridiculously rare. At least in my region, with some features it would happen but of several hundred shops I deleted/retagged none qualified.

what about the rest of my suggestion, which was the main point? Do you have an opinion yet if you would rather prefer potential data loss, or potentially obsolete data?

No strong opinion, both require maintaining long lists of tags and both have negative consequences. Also: it is one more negative consequence of mixing shops and buildings as one object.

[westnordost]

Also: it is one more negative consequence of mixing shops and buildings as one object.

start_date sends his regards.

I chose a negative list rather than a positive list because the former would be, I believe, unmaintainably long.

[westnordost]

So is there anything left to do or is it ready to merge? @matkoniecz , you requested changes?

[westnordost]

Generally, it shouldn't be a blocker to merge a PR only even more improvements can be made. This could be a separate PR, right?

[westnordost]

Looked at the PR again and I will consider to add the suggested after the merge

[westnordost]

See my commit, I added some more things that should not be deleted.

[TurnrDev]

Looks good! Thank you!

[westnordost]

Unfortunately, I noticed an issue which likely makes it necessary to revert this PR:

What if a surveyor already tagged the new shop, but did not remove the disused:* tags? Then, StreetComplete users are asked this quest and whent they answer this, all the tags for the new shop are removed!
This shop for example: https://www.openstreetmap.org/node/3672068811#map=19/53.57618/9.95118

Of course it can be considered an error that the disused:* tags were not removed. But it is just as well an error if any shop-related tags are not removed when retagging a shop to disused:shop=* - the situation this PR tried to fix, by clearing the tags again when a new shop appears.

Of the two possibilities,

1. not removing wrong leftover tags from the old shop or
2. removing recently surveyed tags for the new shop

the latter sounds more destructive.

[TurnrDev]

Surely the fix is it change the query for what qualifies to something like (disused:shop=* and (!shop or shop=vacant))?

[TurnrDev]

https://www.openstreetmap.org/node/3672068811 is bad anyway. Those disused tags should be removed