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

Highlight invalid field values #2836

Open
GerdP opened this issue Nov 20, 2015 · 12 comments
Open

Highlight invalid field values #2836

GerdP opened this issue Nov 20, 2015 · 12 comments
Labels
field An issue with a field in the user interface validation An issue with the validation or Q/A code

Comments

@GerdP
Copy link

GerdP commented Nov 20, 2015

If I got it right, version 1.8.0 will not so easily produce
these invalid tags now, see
#2809

Would it also be possible to ask the mapper to correct existing
invalid tags ?
Example:
If I change a highway with oneway="yes;no" or "surface=asphalt;gravel"
could iD ask me to correct the wrong tags?
Something like a popup with the text
"If you are local, please try to correct (improve) the attribute xyz before saving"

@bhousel
Copy link
Member

bhousel commented Nov 20, 2015

Thanks for the suggestion, but I'd prefer for users who are interested in OSM data quality and cleanups to use dedicated tools like Keepright or Osmose.

While I don't know of any actual studies done, I suspect that there is a kind of 1% rule in OSM participation. E.g. for every 1000 casual mappers, we might get 100 who stick around as power mappers (creators), 10 who focus on cleanup (editors), and 1 who actually contributes code (creators). These numbers are completely made up, but they feel about right to me.

iD's goal is to widen the mouth of this participation funnel, so we don't want to turn away the casual mappers by prompting them to cleanup tags (a task that, while important, relatively few OSM participants are interested in). I would rather get them addicted to mapping, knowing that a handful of of them will convert into users that will take ownership of the map where they live and keep it clean using a dedicated QA tool.

So here are some things that I would support in iD:

  1. I mentioned before that I think it is reasonable for iD to have a special "warning" style for UI fields that have a weird value in them. One common way I have seen this implemented in other systems would be if we support a validation regex in the field.json schema. For a lot of fields, it might just be a simple /^;/ - so that if a field contains a weird value we could gently say "here's something that looks funny" rather than interrupting the user to ask them tagging questions that they can't answer.
  2. Another idea I've mentioned before would be adding Keepright or Osmose issues as toggleable layers in the Map Data panel. Both of these tools have APIs from where we could pull a list of QA issues in the current view and display markers for them. By the way - both of those tools are very good at what they do, but not particularly user-friendly. They are open source projects that need some love and there are many opportunities for someone who cares a lot about OSM data quality to step up and build a better tool for it.

I'll leave this issue open for discussion...

@bhousel bhousel changed the title Ask user to repair invalid values Highlight invalid field values Nov 20, 2015
@GerdP
Copy link
Author

GerdP commented Nov 20, 2015

My idea is that a mapper who is using iD is very likely a local.
Many invalid tags like those for surface or maxspeed or name are
hard to fix without local knowledge. General problems like unconnected
roads, unclosed landuse/natural ways etc. are rather easy to fix for an experienced
mapper while a newbie might have no idea what this means.

@bhousel bhousel added the field An issue with a field in the user interface label Nov 20, 2015
@HolgerJeromin
Copy link
Contributor

But splitting a buggy way and sort the right tags to the parts is probably not easy, too

@GerdP
Copy link
Author

GerdP commented Nov 22, 2015

Yep, probablly that would need support by iD. In JOSM I check the history of the way to find the
changeset which introduced the buggy tag(s) to find out how the way was mapped before.
I guess that could be coded as well to propose a possible split point (or multiple split points),
but I know that this kind of three way merge is not easy to code.

@slhh
Copy link
Contributor

slhh commented Jan 16, 2016

Thanks for the suggestion, but I'd prefer for users who are interested in OSM data quality and cleanups to use dedicated tools like Keepright or Osmose.

In view of existing Tags I agree. Fail safe validation is likely impossible and iD must not generate warnings on existing Tags which might be wrong. Such wrong warnings in JOSM are a real problem, because even some power mappers tend to simply delete reasonable Tags just because of the validator warning. With the less experieced iD users this would be much bigger problem.
Even if the warning is correct the unexperienced user might choose the wrong method to remove the warning.

Tags added during the current edit session are a completely different thing. In this case a warning that something looks funny doesn't really hurt even if it is possibly wrong. Tags of new objects which have come from existing objects by spitting or copying must not be considered as objects being added during the current session. Therefore tags should be validated (or marked for validation) when they are manually added.

@bhousel bhousel added considering Not Actionable - still considering if this is something we want and removed enhancement labels Nov 1, 2016
@sommerluk
Copy link

A further usecase for highlighting invalid fields is: “Only basic phone number check” (#4338 has been closed in favor of this issue here):

A strikt and full check for phone number formating has been discussed yet at #3495 and #2704. Result: The user should not be forced to use a formatting he is not used to.

A less obstrusive solution might be to not check the full phone number format, but to check only if it actually starts with an international prefix (these things like +225 or +229 or whatever). This would not force the users to use digit separations they are not used to, but it might encourage the contributers to add at least the international prefix.

Data consumers could later strip of every non-digit character and it would be likely (more likely than now) that they get something, that is a valid phone number…

@1ec5
Copy link
Collaborator

1ec5 commented Jan 19, 2019

Another idea I've mentioned before would be adding Keepright or Osmose issues as toggleable layers in the Map Data panel.

KeepRight was added to this panel in #5201, and #5682 tracks adding Osmose.

@sommerluk
Copy link

Could checks like the phone numer check be implemented now using the new validation framework that is available since v2.14?

@quincylvania
Copy link
Collaborator

@sommerluk Yep, see #5866.

@quincylvania quincylvania added the validation An issue with the validation or Q/A code label Mar 17, 2019
@sommerluk
Copy link

Okay. So for phone numbers, I propose checking against this regular expression:

^\+(9[976]\d|8[987530]\d|6[987]\d|5[90]\d|42\d|3[875]\d|2[98654321]\d|9[8543210]|8[6421]|6[6543210]|5[87654321]|4[987654310]|3[9643210]|2[70]|7|1)[0-9 \.-]*$

Maybe add support for multiple, semicolon-separated values? The corresponding wiki page mentions that this is used “sometimes”.

@sommerluk
Copy link

This only controls if the phone number starts with an international prefix, and the rest of the number is simply free style (allowing arbitrary combinations of digits and separators like space or hyphen).

@quincylvania quincylvania removed the considering Not Actionable - still considering if this is something we want label Nov 11, 2019
@sommerluk
Copy link

Any news about phone number checking here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
field An issue with a field in the user interface validation An issue with the validation or Q/A code
Projects
None yet
Development

No branches or pull requests

8 participants