better deal with bad OSM data for OH

[matkoniecz]

fixes #3718 and fixes #2712

now old opening hours will be asked only when on objects eligible for new opening hours
and for some additional limited cases when opening hours are tagged already and become old such as parks, barriers, toilets

opening PR as I have feeling that it could be implemented in a bit better way

[peternewman]

Sticking my comment in a more useful place, I think this wants applying to CheckOpeningHoursSigned too, which could have the same issues.

[matkoniecz]

https://github.com/streetcomplete/StreetComplete/blob/master/app/src/main/java/de/westnordost/streetcomplete/quests/opening_hours_signed/CheckOpeningHoursSigned.kt has far less complex filtering

Not sure is such elaborate filter useful there.

[westnordost]

Overall, I am somewhat "meh" about this PR. In #3718 @Strubbl clearly stated that the opening hours quest would only be shown in error if the map data is actually wrong.

So if the quest asks if the opening hours for that disused shop are still correct, the user can simply answer "doesn't exist anymore" and StreetComplete will just fix the problem by tagging the disused shop as a disused shop, removing opening hours etc.
So, I don't really see the issue.

[matkoniecz]

clearly stated that the opening hours quest would only be shown in error if the map data is actually wrong

While I agree that in general StreetComplete is allowed to fail in case of invalid data as it is impossible to handle all possible cases of how data can be broken, I think it is fine to filter out some of such cases. And this case the change seems to be worth it.

In such cases survey with SC is a pure waste of time - of SC mapper and by whoever is processing notes.

As person who is really involved in processing notes - such notes are quite irritating.

Technically there is some minimal benefit as it results in removal of opening_hours from a closed shop, but benefit of that is really minimal.

And time of SC mapper is definitely wasted as this problem can be trivially detected automatically.

So if the quest asks if the opening hours for that disused shop are still correct, the user can simply answer "doesn't exist anymore" and StreetComplete will just fix the problem by tagging the disused shop as a disused shop, removing opening hours etc.

I just tested with https://www.openstreetmap.org/node/3087666570 and SC simply started opening a note rather than retag anything (which may be a separate issue)

---

Note that this problem happens for disused shops when user has "is this shop still vacant" below opening hours or when it is disabled.

So it happens only in cases when user:

• disables "is this shop still vacant" quest
• moves quest order so OH quest is above vacant quest
• hides specific "is this shop still vacant" quest instead of answering it

---

I have not yet reviewed code suggestions, if basic idea of PR is rejected then polishing code makes no sense.

---

I created https://josm.openstreetmap.de/ticket/21832#ticket

[westnordost]

I just tested with https://www.openstreetmap.org/node/3087666570 and SC simply started opening a note rather than retag anything (which may be a separate issue)

Oh right, because the app first checks if the node in question is some kind of shop, and a disused:shop is rightly not detected as "some kind of shop". Makes sense, we don't want to potentially retag things that are clearly not a shop as another shop or vacant shop, plus that "some kind of shop" check is used in other places too.

[matkoniecz]

Overall, I am somewhat "meh" about this PR.

But would you be acceptable to merge it (as long as ugly code issues are solved)?

[westnordost]

Yes, because #3719 (comment)

[westnordost]

Hmm, unless we treat disused shops and amenities as "some kind of shop" only for the intent of replacing a shop. Would make sense too

[westnordost]

isKindOfShopExpression already allows a prefix to be added, so the replace-shop functionality could look for isKindOfShopExpression() + "shop = vacant" + isKindOfShopExpression("disused"), the same way as CheckShopType does. I think this is the better solution at least to solve this case.

[matkoniecz]

I think it may be useful in general, maybe with some exceptions.

Correctly tagged disused:shop=* will be visible and useful for orientation, so it would make sense also in other contexts.

[peternewman]

https://github.com/streetcomplete/StreetComplete/blob/master/app/src/main/java/de/westnordost/streetcomplete/quests/opening_hours_signed/CheckOpeningHoursSigned.kt has far less complex filtering

Yes, although the flip side of that is it's currently only showing the quest for:

StreetComplete/app/src/main/java/de/westnordost/streetcomplete/quests/opening_hours_signed/CheckOpeningHoursSigned.kt

Line 36 in 6aa3fb1

and (name or brand or noname = yes or name:signed = no or amenity = recycling)

It would be nice to also cover the leisure and other amenities from here too.

Not sure is such elaborate filter useful there.

The more practical thing may be that there only seem to be 50 odd disused things tagged with opening_hours:signed:
https://taginfo.openstreetmap.org/keys/opening_hours:signed#combinations

[westnordost]

Done, thanks!

[peternewman]

Shouldn't we drop recycling from here too?

Also should this either be ~, or can the one in AddOpeningHours be = too?

[matkoniecz]

Good catches!

Fixed = vs ~ in 0848d98

amenity = recycling and recycling_type = centre should be either added to name quest or allowed to have no name in opening hours quest, not checked which would be preferable.

Or maybe in addition also removed from opening hours quest, is there distinction between recycling centres that allow walk ins and ones that are factories closed to typical people?

[peternewman]

amenity = recycling and recycling_type = centre should be either added to name quest or allowed to have no name in opening hours quest, not checked which would be preferable.

The recycling_type = centre bit isn't in the CheckOpeningHoursSigned code though.

I'm guessing you meant "rather than not checked"?

Or maybe in addition also removed from opening hours quest, is there distinction between recycling centres that allow walk ins and ones that are factories closed to typical people?

The recycling centres near me which allow walk/drive in from the public are normally council things which also accept waste too, would they be tagged like that not as something else? We used to call them a dump or tip, but they didn't used to recycle quite so much.

[matkoniecz]

The recycling_type = centre bit isn't in the CheckOpeningHoursSigned code though.

Yes, but asking for extremely rare cases where this tag was manually placed on containers is also fine

[matkoniecz]

https://wiki.openstreetmap.org/wiki/Tag:recycling_type%3Dcentre

a public recycling centre

So asking for opening hours is fine