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

Map Maintenance: Handling of fixme=name and suspicious combinations of name=*+noname=yes #5934

Open
kmpoppe opened this issue Sep 29, 2024 Discussed in #5928 · 22 comments
Open
Assignees

Comments

@kmpoppe
Copy link
Collaborator

kmpoppe commented Sep 29, 2024

Discussed in #5928

Originally posted by kmpoppe September 26, 2024

Situation

There are currently ~160k objects tagged with fixme=name.

Understandably, SC currently ignores fixme tags completely - IMHO most values of that key would be impossible to survey without deeper investigation.

Yet, fixme=name seems like a very straight-forward request: "I (the last mapper) was unable to determine what the name of this feature is, please help out".

Proposal

I therefore would suggest for SC to work the following way:

a) remove fixme=name (if present) when the user answers quests/road_name or quests/place_name with either a real name or noname=yes (because otherwise, things like this (noname=yes set, fixme=name persited) or this (name set, fixme=name persisted) happen)

b) the aforementioned quests should force a survey of the name when a fixme=name is older than one year to get rid of it, because all viable answer paths from there (name=*, noname=yes or changing the type of road that doesn't get asked for a name again) lead to a situation where the fixme is no longer relevant.

c) FIXME=name, Fixme=name, fixme=name?, Fixme=Name, ... could be handled the same way.

SCEE

I do not think that this option should only be something relevant for SCEE, because the handling is purely under to hood and there's no expert knowledge necessary to answer those already existing quests.

AE

An automated edit of things that are "clear cut" as proposed by @matkoniecz is planned, SC's tagging cleanup is nevertheless needed for remaining instances of fixme occurances.

@kmpoppe kmpoppe self-assigned this Sep 29, 2024
@westnordost
Copy link
Member

Why not always ask for the name if fixme=name is set?

A note regarding implementation: What if the name is already (partly) set? I am thinking about multilingual names. The previous name(s) should also be shown in the form, if they aren't, yet.

@kmpoppe
Copy link
Collaborator Author

kmpoppe commented Oct 2, 2024

Why not always ask for the name if fixme=name is set?

Makes sense

A note regarding implementation: What if the name is already (partly) set? I am thinking about multilingual names. The previous name(s) should also be shown in the form, if they aren't, yet.

Will have to check how that works.

@kmpoppe
Copy link
Collaborator Author

kmpoppe commented Jan 8, 2025

Obviously neither an AE nor MapRoulette seem to be favoured by anyone, so I'm binning that idea. Using SC to remove the tag when a name is changed still seems like a good idea to me.

@mnalis
Copy link
Member

mnalis commented Jan 8, 2025

Using SC to remove the tag when a name is changed still seems like a good idea to me.

Agreed! When SC users answers a name quest, fixme=name (and variations list fixme=name? etc) tag should be removed.

@kmpoppe
Copy link
Collaborator Author

kmpoppe commented Jan 11, 2025

A note regarding implementation: What if the name is already (partly) set? I am thinking about multilingual names. The previous name(s) should also be shown in the form, if they aren't, yet.

@westnordost I was playing around with the data a bit today. I tried just setting the title for the normal "LocalizedNameForm" to "please confirm the name of this ...", and have the user actively enter or select the name again. Does that feel strange? If so, I'd need to somehow a) argument the current localized names into the form and b) have them selected / input as the default value. Any tips on how that would be achieved the easiest?

@westnordost
Copy link
Member

You mean if all the names are actually already present, but there is a fixme=name floating around?

@kmpoppe
Copy link
Collaborator Author

kmpoppe commented Jan 11, 2025

Yes, whereby all could also mean only one.

If I want to get rid of fixme=name+name=* as a special case of the name quest(s) being asked when fixme is present.

@westnordost
Copy link
Member

Hmm, I am not sure if the form currently supports pre-filling it with pre-existing data. If it does, then your proposed solution will work, I'd say. For the opening hours re-check quest, there is a special UI for that, for the localized names, there surely isn't.

Alternatively, you could also just show the form blank, i.e. ignore pre-existing names. This is consistent with many other "re-check" quests, where the user is just asked for the current situation on-site, ignoring (i.e. not show to the user) how it is tagged currently. Implementation-wise, this would be the easiest solution. And I really don't know if it is worth it to implement for the rather special case of fixme=name + name=*.

Alternatively alternatively, this case could also be ignored, i.e. not trigger the quest. So that quest would only trigger if name is absent and clear any fixme when tagging.

@mnalis
Copy link
Member

mnalis commented Jan 12, 2025

Alternatively, you could also just show the form blank, i.e. ignore pre-existing names.
And I really don't know if it is worth it to implement for the rather special case of fixme=name + name=*.

Agreed, probably not worth special casing. There are about 8k such combinations, and about 27k StreetComplete users. So, assuming random distribution, only every 3rd user will ever to enter the name from scratch once, before all the tags are fixed. So the complexity here is IHMO unneeded (especially as hopefully that combination will not be increasing exponentially).

@kmpoppe
Copy link
Collaborator Author

kmpoppe commented Jan 12, 2025

Thanks guys!

@kmpoppe
Copy link
Collaborator Author

kmpoppe commented Jan 22, 2025

Before I get slightly annoyed at myself: @westnordost are tags in the elementFilter case-sensitive? So, do I have to make the filter include fixme, FIXME, Fixme and FixMe or is fixme enough to match all of those weird variants people have tagged over the decades?

@matkoniecz
Copy link
Member

matkoniecz commented Jan 22, 2025

I would support fixme, maybe FIXME and ignore such extra ones. If I would do anything with them it would be retagging to fixme (expecting anything to support FiXmE or Fixme is silly)

@kmpoppe
Copy link
Collaborator Author

kmpoppe commented Jan 22, 2025

I would support fixme, maybe FIXME and ignore such extra ones.

There are currently 151k fixme=name and 1.5k FIXME=name, the others have even more miniscule numbers. Nonetheless, the question remains whether the elementFilter will ignore casing or not, even if I want to support FIXME as well (this is for filtering AND removing as well).

@matkoniecz
Copy link
Member

matkoniecz commented Jan 22, 2025

whether the elementFilter will ignore casing or not

I would just compile it and test on some test case present in OSM data

maybe check relevant tests in repository that test matching

IIRC it is not ignoring case, but I may be mistaken (similarly I expect that surface quests are not asked on HIGHway=residential)

@kmpoppe
Copy link
Collaborator Author

kmpoppe commented Jan 22, 2025

I already designed some testcases, I'll just make sure to check the casing as well. Thanks for your input!

@westnordost
Copy link
Member

It's case sensitive

@kmpoppe
Copy link
Collaborator Author

kmpoppe commented Jan 22, 2025

Is there a performance gain or loss (while applying any "this now has a name" answer), checking if there are any fixme keys in the object's entries before and not directly calling the remove function?

(data\osm\edits\update_tags\StringMapChangesBuilder.kt):

    fun containsFixmeKeys() : Boolean = keys.any { it.equals("fixme", ignoreCase = true)}

     private val fixmeNameValues = listOf(
         "name", "name?", "add name",     "check name",   "confirm name",
         "missing name",  "name unknown", "needs a name", "unknown name",
     )

    fun removeNameFixme() {
        for ((k, v) in entries) {
            if (k.equals("fixme", ignoreCase = true) && v in fixmeNameValues) remove(k)
        }
    }

@westnordost
Copy link
Member

westnordost commented Jan 22, 2025

Note that this is not core functionality of StringMapChangesBuilder and thus shouldn't be in that file. Should be rather somewhere in osm\.

Also, why not check if fixme value contains name if really so many variations should be checked? I don't like hardcoded lists, especially of "free form values". That's a never-ending source of maintenance.

And of course there is performance loss if you do more, but this code looks like it would be negligible.

@westnordost
Copy link
Member

Looking at taginfo, the only thing that really seems worthwhile to check for (it is even documented, not bad) is fixme=name. The rest are just crumbs

@mnalis
Copy link
Member

mnalis commented Jan 22, 2025

Also, why not check if fixme value contains name if really so many variations should be checked? I don't like hardcoded lists, especially of "free form values". That's a never-ending source of maintenance.

While nobody is probably a big fan of hardcoded lists in any case, but in this case I think it is quite warranted. Only certain very specific fixme tags which are (almost) surely ONLY about the name should be matched and subsequently removed after name=* tag is updated. Matching too little is much preferred to matching too much, as we are not only updating name=* tag, but also removing the fixme tag.

That removal of fixme tag should only be done if we are sure that we won't be losing some other information that other mappers have painstakingly added in it.


Details & examples:

There are other situations which would match fixme ~ name, but which should not be handled so, where:

  • missing/suspicious name is just a part of the problem and full solution needs much more than just updating name=* tag. e.g. "check if this is a consulate or a honorary consulate (name does not match consulate= tag)", "geometry to be corrected; watercourse mising; name missing; not a canal", "different name path vs. relation", etc.

    In those cases the fixme tag should definitely not be removed (but modified to only remove only part of it relating to "name", which would require human or AI manually editing it, and definitely not automatically removing valid data!

  • there are situations where it refers to other name tags (e.g. "short_name conflicts with name in the typos of diplomatic office") and not to name=*

  • there are other situation where 4 letters "name" just happens to be a part of the sentence, and which refer to other things (which may even not be be related to any name tag at all, even matched in different languages - e.g. "Možda fali znamenka ispred 862", "L'altezza è stimata grossolanamente", "da verificare il posizionamento non appena saranno disponibili immagini aeree migliori", ...)

All of the above are real-life examples retrieved via overpass https://overpass-turbo.eu/s/1XyD


TL;DR: Thus, I think we should be conservative here and exclusively go only with fixed hardcoded list of known fixme values (as suggested by @kmpoppe), instead of doing partial matches. It is much better to miss a few potential quests here then to incorrectly remove fixme tags which are not ONLY related to incorrect/missing name.

@mnalis
Copy link
Member

mnalis commented Jan 22, 2025

Looking at taginfo, the only thing that really seems worthwhile to check for (it is even documented, not bad) is fixme=name. The rest are just crumbs

There is a lot less of them, true, but checking a few of them (mentioned above) does seem to add up to about 10% of the use cases of just main fixme=name. I understand that we prefer simplicity, but I'd suggest that they are worthwhile to check while this is being implemented anyway, and it does not increase complexity that much.

But if simplicity is absolutely paramount here, then yes, just fixme = name is infinitely better then fixme ~ name

@kmpoppe
Copy link
Collaborator Author

kmpoppe commented Jan 23, 2025

I'll just go down to ~fixme|FIXME = name (filter and removal), we can consider changing that at a later date.

Draft PR to follow when I got all tests working

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants