[PATCH] Delete relations by default when all members are deleted

[Fred73000]

Hi,

IMPORTANT : I'm not using JOSM so don't ask me anything about it (like technical informations).

According to ​this old talk and according to this overpass query

[out:csv(::type,::id,::version,::timestamp,::changeset,::count)]
[timeout:600];
(
  relation[type](if: count_members() == 0);
  relation[!type](if: count_members() == 0);
) -> .all_empty;
rel(br.all_empty) -> .parents;
rel(r.parents) -> .relmembers;
(.all_empty; - .relmembers;) -> .fullempty;
/* (.all_empty; - .fullempty;) -> .empty_but_in_a_rel; */

.fullempty out count;
.fullempty out meta;

/*.empty_but_in_a_rel out count;
.empty_but_in_a_rel out meta;*/

JOSM is the great winner to create empty relations (I mean with no member and not member of another relation = with no geometry therefore invisible) for more than 10 years.

More than 50 000 bad relations were deleted 10 months ago, the list is in this ​file, thousands more in the following months (by me in particular) and today there are still 12 787 empty relations, most created by JOSM.

Juste the ones created today with JOSM =

@type	@id	@version	@timestamp	@changeset
relation	11641763	2	2022-02-02T13:53:15Z	116914653
relation	13751283	1	2022-02-02T13:53:15Z	116914653
relation	13751278	1	2022-02-02T13:52:35Z	116914626
relation	13746202	2	2022-02-02T13:50:01Z	116914514
relation	11372799	2	2022-02-02T13:30:27Z	116913726
relation	11592420	2	2022-02-02T12:43:30Z	116911875
relation	11373145	2	2022-02-02T11:53:23Z	116909851
relation	11641931	2	2022-02-02T11:44:38Z	116909527
relation	13750701	1	2022-02-02T11:21:06Z	116908592
relation	13750702	1	2022-02-02T11:21:06Z	116908592
relation	13283862	2	2022-02-02T10:21:17Z	116906028
relation	13063916	2	2022-02-02T10:07:42Z	116905454
relation	11687436	2	2022-02-02T09:48:46Z	116904641
relation	13366944	2	2022-02-02T07:25:14Z	116898981

Could you do ANYTHING to stop to create these bad datas in OSM ?
There are possibly several reasons for their creations, like I say above, I'm not using JOSM so I don't know why, I just can say how many and how long. Just have a look to the relations in the table above, have a look at some of the around 12 000 others and try to find all the problems to stop this very bad thing.

Best regards

[taylor.smock]

You might want to look at

• #19316: Don't show warning when a new empty relation is deleted
• #20781: Prevent uploading of empty relations

This is most likely partly due to #3423: Warn when deleting a large relation in r4677.

Anyway, the root cause is probably users deleting members in the relation, and not deleting the relation afterwards.

[taylor.smock]

Add table where users can deselect/select relations to delete, when a delete command will remove all members of the relation

[Fred73000]

Hi,

because I'm not using JOSM (see the beginning of this ticket), I would like to ask you a direct question : was the patch 21825 included in a new version of JOSM or not yet ?

Best regards

[taylor.smock]

Replying to Fred73000:

Hi,

because I'm not using JOSM (see the beginning of this ticket), I would like to ask you a direct question : was the patch 21825 included in a new version of JOSM or not yet ?

Best regards

No. I was giving other JOSM developers a chance to look at the patch. We are also currently in between one of the two weekends where a release may happen, so I'm not going to merge the patch this week. The earliest I will merge the patch is March 10th. And I'm going to want to add a test for correctness prior to merging it.

[GerdP]

My 2 cents:
If a user removes all elements of a relation, but not the relation itself this is almost for sure a user error. I don't think that we should presume that the user also wants to remove the relation, we can as well assume that the user doesn't understand the meaning of the relation. In the latter case the user probably doesn't know what to do with the error message and maybe decides to remove the relation(s) as well as "this makes JOSM validator happy".
I think the real problem here is that the user is not willing to throw away their bogus changes.

[taylor.smock]

Replying to GerdP:

My 2 cents:
If a user removes all elements of a relation, but not the relation itself this is almost for sure a user error. I don't think that we should presume that the user also wants to remove the relation, we can as well assume that the user doesn't understand the meaning of the relation. In the latter case the user probably doesn't know what to do with the error message and maybe decides to remove the relation(s) as well as "this makes JOSM validator happy".
I think the real problem here is that the user is not willing to throw away their bogus changes.

Well, we do tell users that they don't need to delete relations. From source:/trunk/src/org/openstreetmap/josm/actions/DeleteAction.java@HEAD:116-142#L116​, "[this] step is rarely necessary [...]."

All the attached patch does is detect that a user is deleting all members of a relation, and offers to get rid of it (the checkbox is enabled by default -- I can change that, or I can whitelist specific relation types like multipolygons or turn restrictions). I really ought to add something that detects the removal of all members from a relation, and offer to undo the removal or delete the relation (as compared to the deletion of all members).

[Fred73000]

I don't know how works JOSM but for me, with my own experience of ID, there are 2 cases :

• the user is on a relation and he deletes (= takes off) all the members from the relation : the relation is now "empty" in JOSM. For me the user clearly wants to kill the relation : the empty relation should be, I think, automatically deleted by JOSM when he send the changeset to osm database.
• the user deletes some nodes, some ways, some relations. Maybe he didn't see these nodes/ways/relations are in a relation and by deleted them the relation will be empty : I think a warning is necessary (maybe he created others nodes/ways and forgot to put them in the relation ?). The warning should be blocking : the user has to do something = to cancel a deletion, to add an element in the relation or to say "yes I know, I want to delete the relation too". If he checked "delete the relation", he may send the changeset to osm database, if he doesn't check, he can't send the changeset, he has to fix the relation before (because an empty relation is a bad data and should not be send to the database).

Best regards.

[GerdP]

There are more cases in JOSM, but in the end the user is always warned when he tries to upload empty relations. The user is probably not understanding the concept of relations when they decide to upload. If we disallow the upload of empty relations we simply force the user to delete the relations, while the better solution could be to undo the changes.

[GerdP]

In 18395/josm:

fix #21825: Delete relations by default when all members are deleted
Patch from taylor.smock:

• Add table where users can deselect/select relations to delete, when a delete command will remove all members of the relation

[GerdP]

Thanks for the patch, it may help in some cases.
I think the real culprit are the presets for relations. They are active when no element is selected, and create empty relations in that case if the user decides to press OK. How else can we explain new empty type=associatedStreet or type=building relations? My guess is the users are either too quick or they simply experiment with presets without understanding the meaning.

[stoecker]

Replying to GerdP:

Thanks for the patch, it may help in some cases.
I think the real culprit are the presets for relations. They are active when no element is selected, and create empty relations in that case if the user decides to press OK. How else can we explain new empty type=associatedStreet or type=building relations? My guess is the users are either too quick or they simply experiment with presets without understanding the meaning.

Hmm, wasn't there a check that relation presets did nothing when no members have been selected?

[GerdP]

No idea. I've tried some older releases down to r9086 and the behaviour is always like today.

[Fred73000]

list of empty relations from 15/02/2022 to 15/03/2022

[Fred73000]

Hi,

I just add a file with the list of the empty relations sent to osm database for one month :

• 1 was created using Vespucci (an issue was created just now on github)
• 2 with Potlatch (an issue still exist on github)
• and 302 with JOSM (that means = more than 3600 a year !)

I hope your patch will be enough to stop this.

Best regards

[stoecker]

Replying to GerdP:

No idea. I've tried some older releases down to r9086 and the behaviour is always like today.

So my suggestion would be to prevent creating relations via presets when no members are selected. Simply disable the button?

[taylor.smock]

Replying to stoecker:

So my suggestion would be to prevent creating relations via presets when no members are selected. Simply disable the button?

If we disable the button when nothing is selected, do we want to automatically add the selected objects as members on relation creation?

[stoecker]

Wasn't this the case?

Sorry, I'm not sure what I implemented and what I only planned to implement :-)

[stoecker]

That's what I planned:

• describe the members in the XML
• automatically create and assign selected elements as members when non relation
• assign only tags when a relation is selected (and maybe check members)

I'm pretty sure that "member checks" never were implemented.

[taylor.smock]

It does look like we automatically add selected items when creating relations from presets, but not when creating relations from the relations pane.

Also, I'm going through the list of relations given (version 2+) to see what the problems there are. So far, it looks like r18395 should fix a good chunk of those. But not all.

[taylor.smock]

OK. I haven't gone through all of them, but ~1/3 were v1 (so preventing relations from being created without members would fix that), all members being removed (but not necessarily deleted) account for 89, and all members being deleted account for 68.

Note: I only went through 158 (92 were v1, which means they were created without members).

Maybe we should just implement a check when saving the relation to check and see if there are no members. Maybe even replace OK with Delete if there are no members.

I'll take a look into that.

[taylor.smock]

I've traced SavingAction:56-58#L58​ back to r1431, and I think that the original author (xeen) probably intended it to be || instead of &&. But I don't know.

Anyway, in the patch I'm uploading, I've changed it from && to || and modified the tag check to see if there is a type tag instead of whether or not any tag exists -- the wiki page on relations indicates that they must have at least a type tag.

Let me know if the patch makes sense to you (especially from a UI standpoint -- I think it makes sense, but users might disagree).

Pretty much what the new patch does is switch between delete and ok actions based off of whether or not the user has added all required information for a relation in OSM. Exception: when the relation is new, it does not switch between the two actions.

[taylor.smock]

Prevent users from saving invalid relations

[stoecker]

I'd move the check into the relation class as isUseful() or isValid() or something alike. If possible replacing all three cases, if not at least for the SaveAction case.

Java is object oriented, so functions which belong to the object also should be in the object.

[taylor.smock]

I hadn't done that since most of those locations didn't have the new relation (yet). And I kind of was trying to keep the patch minimal until I got some UI feedback. But yes, having the same logic in multiple places isn't great.

[taylor.smock]

Add validate and isUseful functions to IRelation, use both in GenericRelationEditor -- isUseful controls whether or not the relation will be deleted and validate provides live validator feedback.

[GerdP]

Replying to taylor.smock:

I've traced SavingAction:56-58#L58​ back to r1431, and I think that the original author (xeen) probably intended it to be || instead of &&. But I don't know.

Yes, the original code looks strange.
I am surprised that we already have a check for this. Why do we bother about empty relations, but not about untagged nodes or ways without parents? Or do we? My understanding was that we allow anything that the OSM API allows.

[stoecker]

Replying to GerdP:

I am surprised that we already have a check for this. Why do we bother about empty relations, but not about untagged nodes or ways without parents? Or do we? My understanding was that we allow anything that the OSM API allows.

We support anything as long as it makes sense in at least a possible case (maybe outside OSM). I can't think of any case for empty relations, so there is no harm in preventing them.

[GerdP]

OK. What about a new node without tags and parents? In what situation can it make sense?

[stoecker]

Replying to GerdP:

OK. What about a new node without tags and parents? In what situation can it make sense?

When using JOSM as a GIS software to display or mange a set of nodes without further information.

[GerdP]

Ah, sorry, I misread the patch. I meant the case that such a node is uploaded to OSM, but the patch just prevents the creation of empty relations.
I've now tried it and I think there is something wrong. Steps to reproduce:
1) Create new relation by pressing the + button in the relation list
2) add tag type=multipolygon (or any other valid type=*)
3) click on the icon for the diskette: Apply the current updates
Expected result: Either no change or a message that the relation cannot be saved.
Result: The type tag is removed.

[taylor.smock]

The patch does prevent the creation of empty relations, along with giving the user live feedback as they are editing the relation (note: does not check for duplicate relations at this time).

I hadn't been checking the Apply the current updates button. Anyway, it looks like (on my machine) the Apply the current updates button was only being updated when I change a role in the member list.

[GerdP]

I guess the Apply the current updates action should be disabled in my scenario.

[taylor.smock]

Replying to GerdP:

I guess the Apply the current updates action should be disabled in my scenario.

That is what I did with attachment:21825.4.patch​ (see GenericRelationEditor.java#L280-283, ApplyAction.java).

With that being said, I'm not a big fan of the getChangedRelation function I added in IRelationEditorActionAccess.java, but I didn't see a better place to put it.

[GerdP]

Yes, sorry, didn't see the new patch before I wrote my comment. Works better with 21825.4.patch.

[GerdP]

Some problems:
1) If you load a relation in the relation editor and remove all members, it doesn't suggest to delete the relation.
2) Messages from e.g. MultipolygonTest are not updated when the member list is changed.
3) I don't like the idea to run the validator like that. The relation editor is more or less unusable with complete complex multipolygons, esp. with Lake Huron: ​https://www.osm.org/relation/1205151.
Remember that the editor is also used to visualize the member list.

[taylor.smock]

1) This was an easy fix -- I had made the assumption that getRelation would return an object that had been modified to match the state of the editor, and just returned that in getChangedRelation.
2) This was actually related to (1)
3) I've moved the validation call into the worker thread. It would probably be better to move it to a ForkJoinPool though, to avoid blocking other stuff (although I don't know what the other stuff would be). TBH, that relation had issues on JOSM pre-patch as well (several seconds to select a different member immediately after selecting a different member when the entire relation is in view, mostly due to painting the selection in the mapview -- this can be fixed by zooming in).

EDIT: Also, don't try deleting all members of the Lake Huron relation -- it proceeds to spend a lot of time in Way#hasIncompleteNodes. I almost feel like this should be cached (the short flags field has space for two more flags, I believe).

[taylor.smock]

Move validation to worker thread, always set relation members/tags.

[GerdP]

I think the code to run validator should be removed. The tests are triggered far too often and I don't see any need for this to solve this ticket. I see long delays when I e.g. remove a node from a way that is a member in a complete multipolygon relation.

[taylor.smock]

Replying to GerdP:

I think the code to run validator should be removed. The tests are triggered far too often and I don't see any need for this to solve this ticket. I see long delays when I e.g. remove a node from a way that is a member in a complete multipolygon relation.

This is actually from updating the button states, and trying to ensure that we are using the code in IRelation for checking usefulness. Not from the validation itself (as in, I could remove the validation, and there would be no impact on the UI freeze).

The actual problem appears to be from the code updating connectivity. I'll have to rework the listeners updating the buttons to avoid being called in the loop (MemberTableModel#wayNodesChanged). Most of it is from creating thousands of new relations. :(

Edit: See MemberTableModel.java#L178​ -- I'm looking into replacing the for loop with a single table update event. This decreases the UI freeze on my machine from ~20s to <1s. And the connection column appears to properly update. I'm checking to see if there are any issues with the change, but I'm not seeing any -- I'll be doing a more in-depth dive to all the listeners that are called by it in the next hour or two.

[taylor.smock]

Standalone patch: Avoid loop in MemberTableModel#wayNodesChanged. This decreases UI freeze from ~20s to <1s for ​Lake Huron.

[taylor.smock]

Measurements (async profiler) for attachment:21825.avoid_loop_MemberTableModel_wayNodesChanged.patch​:

Original code (no patches, jar downloaded from website, 30s for update on node delete, profiling was ~36s long, EDT blocked for ~30s):

• CPU
  • EDT: 69.31% of all
    • MemberTableModel#wayNodesChanged: 65.89% of all, 95% of EDT
  • G1: 21.09% of all
• Memory
  • EDT: 80.75% of all
    • MemberTableModel#wayNodesChanged: 66.93% of all, 82% of EDT
    • MapView painting: 13.07% of all, 16.18% of EDT
  • AutoSaveTask: 19.16% of all

New code (~6s for update on node delete -- most of time is from moving mouse to start/stop profiling, also has validator changes, EDT blocked for <1s at any time):

• CPU
  • EDT: 16.99% of all
    • MemberTableModel#wayNodesChanged: 1.98% of all, 11.65% of EDT
  • G1: 6.31% of all
  • Validation: 68.41% of all
• Memory
  • EDT: 97.76% of all
    • MemberTableModel#wayNodesChanged: 6.04% of all, 6.17% of EDT
    • MapView painting: 79.59% of all, 81.41% of EDT
  • Validation: 1.48% of all

EDIT: No listeners (at least in core JOSM) actually check for which column is modified. Or even what changed in the table.

[GerdP]

Yes, the patch helps but I still don't see a need to run the tests when I change the selection, esp. not the rather complex MultipolygonTest.
I'd prefer to minimize the patch to really check for empty relations and maybe open new tickets for the rest.

A general problem with the displaying of the TestError:
I see e.g. "Multipolygon rings share node with 2 more problems", a rather confusing message. I tried to click on that message to get more info. Same problem with a text like "Intersection between multipolygon ways with 1 more problem"

Other problems:
When I create an MP, open it in the relation editor and change it to self-intersect by moving a node the error texts are not always refreshed. The moved nodes stays selected and the tests are not re-run unless the node is deselected. When I use Undo/Redo to revert the move the test results are not properly refreshed.

I think the major problem here is that the relation editor is not meant to be in sync with the data, thus it causes all kinds of strange effects when it reacts on some events but not on others.

[taylor.smock]

Replying to GerdP:

Yes, the patch helps but I still don't see a need to run the tests when I change the selection, esp. not the rather complex MultipolygonTest.

The tests aren't actually run when you change the selection in the member list (or at least, it didn't hit my breakpoint). They are run when you change the selection in the TagEditorModel (as in, the TagEditorModel fires a table changed event regardless of whether or not the table changed). This does seem like a bug -- I'll open a ticket for this. It will probably take quite a bit of investigation (the code hasn't been touched since 2010, so who knows what is depending upon the current behavior).

I'd prefer to minimize the patch to really check for empty relations and maybe open new tickets for the rest.

I'll go ahead and do this. See #21950, #21951.

A general problem with the displaying of the TestError:
I see e.g. "Multipolygon rings share node with 2 more problems", a rather confusing message. I tried to click on that message to get more info. Same problem with a text like "Intersection between multipolygon ways with 1 more problem"

Fair enough -- I was trying to indicate to the user that once they fixed the current problem, they would probably have more issues to fix. Not have a comprehensive list of issues (caveat: it always shows the more severe errors by preference).

Other problems:
When I create an MP, open it in the relation editor and change it to self-intersect by moving a node the error texts are not always refreshed. The moved nodes stays selected and the tests are not re-run unless the node is deselected. When I use Undo/Redo to revert the move the test results are not properly refreshed.

I think the major problem here is that the relation editor is not meant to be in sync with the data, thus it causes all kinds of strange effects when it reacts on some events but not on others.

It is kind of a best effort thing. We already try to keep in sync with node changes for the connectivity column.

[GerdP]

I see no need to introduce IRelation.validate() and the changes in Relation here. IRelation.isUseful() is missing a @since xxx
I am also not convinced that a relation without type=* tag should be deleted. This ticket is about empty relations, let's stick to that.

[GerdP]

OKay for me.

[taylor.smock]

Restrict patch to only empty relations with no tags, add missing @since xxx

[taylor.smock]

Replying to GerdP:

OKay for me.

I'll go ahead and apply the patch tomorrow, to give other people a chance to give feedback.

[taylor.smock]

In 18413/josm:

fix #21825: delete non-useful relations

This offers to delete relations when users have either removed all
members or all tags.

[Fred73000]

list of empty relations from 15/03/2022 to 15/04/2022

[stoecker]

Seems the changes helped. But still a few even with r18427.

[Fred73000]

Hi,

I just add a file with the list of the empty relations sent to osm database for one month :

• 3 was created using Vespucci
• 1 with a reverter plugin of JOSM
• and 233 with JOSM

The bad news is that 8 were created with the version 18427 which means with the patch.

Best regards

[Fred73000]

@stoecker : yes, it still looks better

[Fred73000]

list of empty relations from 16/04/2022 to 15/05/2022

[Fred73000]

Hi,

I've just added a file with the list of the empty relations sent to osm database for the last month (between April 15 to May 15) :

• 14 were created using Vespucci
• 6 with osmtools
• 2 with a reverter plugin of JOSM
• and 407 with JOSM (that means = almost 5000 a year !)

Clearly the worst month for a long time.

For me there are now 2 problems =

• 64 were created with the last version of JOSM. It means almost 800 a year, clearly too many. 22 are new relations (version 1) = created empty. 42 are old relations (versions 2 to 19) which became empty because of JOSM.
• 343 were created with an old version of your software : very strange that, after one month and half, so many people use obsolete versions of it (the worst is version 14178, almost 4 years old).

Best regards

[taylor.smock]

Replying to Fred73000:

For me there are now 2 problems =

• 64 were created with the last version of JOSM. It means almost 800 a year, clearly too many. 22 are new relations (version 1) = created empty. 42 are old relations (versions 2 to 19) which became empty because of JOSM.
• 343 were created with an old version of your software : very strange that, after one month and half, so many people use obsolete versions of it (the worst is version 14178, almost 4 years old).

Right now, we are just checking if the relation has no tags and no members. I think this was to handle usage outside of OSM where a relation with no tags might be ok. Or a relation with no members.

For the old JOSM versions, we cannot do much (the MOTD has a prompt for users to update). Some Linux distributions carry very old versions of JOSM. But that user probably ran the installer/downloaded the jar file and has never updated (and I don't know how difficult it would be for us to have an auto-updater, outside of the JNLP).

[Fred73000]

Hi,

For relations without tags, I don't know. Currently, there are 3894 relations like that, I will look if it makes sense for some of them.

For empty relations, it is clear : it is always bad datas, see ​https://wiki.openstreetmap.org/wiki/Empty_relations.

Best regards.

[Fred73000]

In fact, for relations without tags it is also very clear : a relation should have at least a tag "type" (see ​https://wiki.openstreetmap.org/wiki/Relation). So relations without tags are also bad datas.

[taylor.smock]

Replying to Fred73000:

In fact, for relations without tags it is also very clear : a relation should have at least a tag "type" (see ​https://wiki.openstreetmap.org/wiki/Relation). So relations without tags are also bad datas.

From comment:24:

We support anything as long as it makes sense in at least a possible case (maybe outside OSM). I can't think of any case for empty relations, so there is no harm in preventing them.

I was originally checking for type and for members. Since a relation outside of OSM may not need a type, that got canned. And there might be some non-OSM cases where a relation with no members is OK (i.e., someone decides it would be a good idea to put defaults for a dataset in a relation with no members). So the check for no members got canned. No tags and no members === useless relation, no matter what. We probably could add a OsmApi.isDefault check, but that would make testing messy, at best.

[GerdP]

I think the question is if we should add code which refuses to upload such data to OSM. We could also check untagged nodes and ways without a parent object.

[stoecker]

Replying to GerdP:

I think the question is if we should add code which refuses to upload such data to OSM. We could also check untagged nodes and ways without a parent object.

No. If such objects should be refused, then the OSM API should do that.

The worst which we should do is show these as error in the upload validator step.

[skyper]

Replying to taylor.smock:

I was originally checking for type and for members. Since a relation outside of OSM may not need a type, that got canned. And there might be some non-OSM cases where a relation with no members is OK (i.e., someone decides it would be a good idea to put defaults for a dataset in a relation with no members). So the check for no members got canned. No tags and no members === useless relation, no matter what. We probably could add a OsmApi.isDefault check, but that would make testing messy, at best.

In fact, the relation editor does not allow to create a relation without member or tag, now. Is there an option to allow both cases?

[taylor.smock]

Replying to skyper:

In fact, the relation editor does not allow to create a relation without member or tag, now. Is there an option to allow both cases?

I have to correct myself. I guess I was looking at the wrong patch (either that or I saw || instead of &&). Anyway, in r18413, we use !this.isEmpty() && this.hasKeys() to determine usefulness. So if the relation is empty or has no tags, we presume it to be useless. So the relation editor will ask you to delete the relation.

Anyway, I'm now confused. How the heck are new versions uploading broken relations? At this point, we're going to need workflows that create the broken relations. Since I don't know how they are happening. Maybe a plugin?

[skyper]

Replying to taylor.smock:

Anyway, I'm now confused. How the heck are new versions uploading broken relations? At this point, we're going to need workflows that create the broken relations. Since I don't know how they are happening. Maybe a plugin?

Well, I can delete the tags in Tags/Memberships panel and also any membership an object has.

[skyper]

Anyway, I am still not convinced of all the changes in this ticket. If users dismiss validator errors on upload they will find ways to get around other restrictions, too.

[GerdP]

We could ask users to give a special comment on upload when validator shows an error or let JOSM show the crash report to get some info from those users ;)

[skyper]

Replying to GerdP:

We could ask users to give a special comment on upload when validator shows an error or let JOSM show the crash report to get some info from those users ;)

We need to filter on validator errors from core as some external validator rules have error warnings, too, despite not being an error in all cases.

[GerdP]

Forget it, I was just kidding. My guess is that we have users who simply don't understand the concept of relations and possibly also are not fully capable to read the error messages (bad translations or foreign language). Empty relations might be the result of poor conflict handling which is really hard to handle.
I've tried to revolve conflicts once and finally gave up and dropped all my changes. I am pretty sure other users simply don't give up and upload what they consider important work.

[taylor.smock]

Replying to GerdP:

We could ask users to give a special comment on upload when validator shows an error or let JOSM show the crash report to get some info from those users ;)

New changeset tag, explanation_for_ignored_validation_warnings? I think I like that better than doing a crash report. But a crash report might work as well, if we customize it a bit. Maybe something like this:

Validators enabled:
* foo
* bar

Validators ignored:
* foo
* bar
 * w20000 # Not a problem due to foobar
 * n10 # I don't care about the validator

----

standard crash report?

Replying to skyper:

We need to filter on validator errors from core as some external validator rules have error warnings, too, despite not being an error in all cases.

If we do the changeset tag, it might not be as critical (I know I'd like to know why users ignore some of my custom validation warnings), but I would definitely exclude any external mapcss files. At least for now.

[taylor.smock]

Replying to GerdP:

Forget it, I was just kidding. My guess is that we have users who simply don't understand the concept of relations and possibly also are not fully capable to read the error messages (bad translations or foreign language). Empty relations might be the result of poor conflict handling which is really hard to handle.

Knowing about the bad translations might be worth it.

I've tried to revolve conflicts once and finally gave up and dropped all my changes. I am pretty sure other users simply don't give up and upload what they consider important work.

Someone really ought to look into the conflict stuff sometime. I've seen way too many people take their changes and break relations. We probably ought to consider members "OK" if the user didn't make any changes directly connected to them.

The someone will probably be me, unless someone else gets to it first.

[GerdP]

One change that might be done: Add a changeset tag that shows that the user really has been warned about the error and still decided to upload. We cannot be sure about that because validator might be disabled (maybe only partly).
No idea if this is allowed or how to implement it. A user might even decide to place the corresponding error message into the ignore list.

[stoecker]

Replying to GerdP:

One change that might be done: Add a changeset tag that shows that the user really has been warned about the error and still decided to upload. We cannot be sure about that because validator might be disabled (maybe only partly).
No idea if this is allowed or how to implement it. A user might even decide to place the corresponding error message into the ignore list.

I'd simply say when validator shows an error and the user proceeds then add a validator_error=yes to the changeset tags. The users have still a chance to remove it at that stage.

[taylor.smock]

Replying to stoecker:

I'd simply say when validator shows an error and the user proceeds then add a validator_error=yes to the changeset tags. The users have still a chance to remove it at that stage.

Related tickets (as far as modifying changeset tags go, and notifying the user that the changeset tags have changed):

• #20025: Changeset tag created_by added to all uploads without informing the user (partially core, partially reverter plugin)
• #22080: UploadHooks modifyChangesetTags should be called prior to showing the user the upload dialog for non-late hooks (core)

The patch for #22080 is in #20025, since #22080 hadn't been filed yet.

[Fred73000]

Replying to taylor.smock:

Since a relation outside of OSM may not need a type, that got canned. And there might be some non-OSM cases where a relation with no members is OK (i.e., someone decides it would be a good idea to put defaults for a dataset in a relation with no members). So the check for no members got canned.

For 2 weeks, I fixed more than 1,000 relations with no tags. Conclusion = 90% were bad datas (I mean useless = only one member, 80% a building or similar like shops). Some were redondant (a relation with no tags, several members but you had a relation with the same members and with appropriate tags). 10% were errors, mostly multipolygons where tags were on the outer way instead of on the relation. More rarely public_transport, boundary. I fixed all these 10% errors and deleted all bad datas.

A relation with no member is invisible because no geometry. So what the useless to "decide" something that nobody, in particular the one who decides it, will see it ? Maybe only usefull for spies who want to hide things in the database...

[GerdP]

Just to mention it: A relation with only one member can be perfectly correct. Typical sample: A ring with natural=coastline is the only member of a multipolygon with natural=bare_rock.
There are several ways in JOSM to create a relation without members. Some of them are probably accidential, e.g. when doing sparse editing.
A user may decide to delete some elements without being aware that these elements can be members of relations. Later, on upload, JOSM will show conflict messages and it is likely that some users don't know how to handle these conflicts correctly and instead of dropping there work because of such a conflict they decide to remove the members from the relation. In that case the correction is often NOT to delete the relation but to undo the modification of that relation. Only expert users know how to do that.

[GerdP]

One rather simple way to produce empty relations is this:
Use Shift+J to "Join overlapping areas", e.g. two landuse=farmland. In some cases this produces not just a single area but a multipolygon with an unintendend small inner because nodes lines where not connected properly. Typically I don't always notice this and only later (after many changes) I will find the unwanted inner. When I simply remove than I have an - unwanted - multipolygon with a single outer way. So, I decide to add the landuse=farmland tag to the outer way and right-click on the line with the membership. When I select "delete" the multipolygon is empty but I see no warning about this. Later, on upload, I get an error message and have to select the relation to delete it. Maybe some users don't understand that this is needed before upload.
OTOH it is also possible that I don't notice that my using of Shift+J produced the multipolygon and since no warning appears for that on upload I may well have created some "useless" multipolygons on my own.

Possible changes:
1) Show notification when Shift+J created a multipolygon
2) If I delete the last member(s) of a relation without using the relation editor a popup might ask to confirm that I want to delete the relation as well.

[Fred73000]

Replying to GerdP:

Just to mention it: A relation with only one member can be perfectly correct. Typical sample: A ring with natural=coastline is the only member of a multipolygon with natural=bare_rock.

Of course, but I'm not talking about general cases, I'm talking about a relation with no tags. When this relation has only one member, a way with the only tags building=yes, it is clearly a useless relation. And 80% of the relations with no tags are of this kind (a building=*, or a shop=*, or a amenity=* and so on).

[Fred73000]

list of empty relations from 15/05/2022 to 15/06/2022

[Fred73000]

Hi,

I've just added a file with the list of the empty relations sent to osm database for the last month (between May 15 and June 15) :

• 1 was created using Vespucci
• 1 with Go Map
• and 185 with JOSM (quite better !)

With the last versions of JOSM (since the patch), we have still relations created empty (for example 14250776, the user is on osm since 2009), relations where members have been removed (for example 1307922, the user is on osm since 2012) and relations where members have been deleted (for example 14167460, the user has almost 8000 changesets).

Best regards

[Fred73000]

relations without tags created in the last 3 months

[Fred73000]

About the relations without tags :

I just added a file with the relations without tags created in osm database in the last 3 months (124).

81 were created with JOSM, the "big winner" again, lol

[skyper]

Overall, I noticed a tendency of less respect and care about the work of other contributors in OSM in general. Especially, throwing away the own changes and trying again, seems to be mostly unacceptable. Sadly, when it comes to reverting, undeleting and/or fixing relations many users fail to do it properly.

Replying to GerdP:

Possible changes:
1) Show notification when Shift+J created a multipolygon
2) If I delete the last member(s) of a relation without using the relation editor a popup might ask to confirm that I want to delete the relation as well.

+1

I still wonder why users do not see or care about the validator errors and warnings.

@Fred73000:
A major help would be to find the underlying cause by asking the individual mappers.

[taylor.smock]

From comment:47 ("407 with JOSM (that means = almost 5000 a year !)" (407 per month, 64 from r18427)), a decrease to 27/month (see comment:71) is a significant improvement.

Anyway, from attachment:notags_res.csv​, and using changesets from ​https://planet.osm.org (changesets-220613.osm.bz2), changesets starting on May 01, 2022:

count	version	Version share	date	comment
3	r14200	0%	2018-08-29	Not even a "tested" release
3	r15937	0.2%	2020-02-26
2	r16538	0.12%	2020-06-02
1	r17084	0.52%	2020-10-05
3	r17702	0.25%	2021-04-01
1	r17833	0.65%	2021-04-28
3	r17919	1.81%	2021-06-02
2	r18004	0.49%	2021-07-12
1	r18118	1.26%	2021-08-02
27	r18303	10.83%	2021-11-01
8	r18360	5.22%	2022-01-03
4	r18387	4.53%	2022-03-07
17	r18427	53.50%	2022-04-05	First release where we started attempting to fix #21825
6	r18463	12.70%	2022-05-30

r18463 is the current tested version. So only 23 of the 81 (or <30%) of the bad relations are from r18427+, which have an uptake of 68% of changesets or 71% of users (r18303+ is ~90% of changesets and users).

Adding on to what I said in comment:55 and skyper said in comment:72, at this point, we'd really like the workflows that cause the empty relations. We've already fixed the source of 90%+ of the empty relations, and I don't want to guess at what is causing the remainder.

[E de Wit]

I was made aware by Fred73000 of an empty relation I created during my changeset 123146597 and did some digging as to why it happened and a possible solution in order to reduce/eliminate these empty relations.

For me it seemed to have happened after putting the map with 'Role verification problem' validations on the ignorelist.
In my area when I get this feedback I cannot do much about said problem without knowing the local situation.
And in my mapping area in the province of Fryslân here in The Netherlands these problems are no easy fix.
To provide better (visual) feedback this map ended up on the ignorelist.
Doing so resulted in also ignoring (unbeknownst to me) the empty relation error which certainly shouldn't have been on my ignorelist.

Knowing this I propose an additional change besides the ones GerdP suggested:
1) Remove the Empty Relations warning from 'Role verification problem' and add it into a seperate map to avoid people putting it on the ignorelist by accident.

[GerdP]

Please can you copy the corresponding entry from the ignore list? It is now in the preferences.xml in entry validator.ignorelist

[E de Wit]

Replying to GerdP:

Please can you copy the corresponding entry from the ignore list? It is now in the preferences.xml in entry validator.ignorelist

Certainly:
Key: 1701 | Value: Role verification problem
Key: 1708 | Value: Role verification problem

When 1708 is removed from to the ignore list the 'Relation is Empty' error appears again.
But when doing so some of the for me unwanted validation warnings are back again as well.

[GerdP]

I see. The number 1708 is used with two different error texts. That's not good and should be fixed. Seems I missed this when I fixed #17567 with r14990 :(

[GerdP]

In 18506/josm:

fix #22187, see #21825: Same error code 1708 for two different problems
I introduced this annoying bug with r14990 and it probably caused lots of empty relations since users where not warned when they ignored the less important message "Role of relation member does not match template expression"

[Fred73000]

list of empty relations from 15/06/2022 to 15/07/2022

[Fred73000]

Hi,

I've just added a file with the list of the empty relations sent to osm database for the last month (between June 15 and July 15) :

• 1 was created using Potlatch
• 1 was created using GoMap
• and 319 with JOSM (not a good month).

With old versions of JOSM (older than 18427), we have 160 empty relations.
With the first patch of JOSM (versions 18427 and 18463), we have 146 empty relations.
With the second patch (versions 18511 and 18513), we have 13 empty relations (created in the last 6 days).

For a few days I asked several users about their changesets :

• when they were using an old version, I only suggested to update JOSM.
• when they were using a version with the first patch, I asked for informations to try to understand these empty relations.

Most answers are "don't pay attention to the warning".
Here some other interesting (maybe) comments :

• ​https://www.openstreetmap.org/changeset/122926111
• ​https://www.openstreetmap.org/changeset/123149821
• ​https://www.openstreetmap.org/changeset/122942980
• ​https://www.openstreetmap.org/changeset/123137060

note = at the end of the file, you have 3 empty relations created with JOSM, empty but children of another relation. I don't know if you use the same process for this kind of empty relations.

Best regards

[Fred73000]

Hi,

107 empty relations (no member AND no tags) created in one changeset : 124554752 (by a user with the description "I am a Mapper and Researcher by profession, working as a Data Officer at Map Kibera")

I will love the day when you prevent this catastrophic outcome even for skilled people.

I would like to understand why you agree to send such bad and useless datas in osm database when it seems so obvious that you should delete these new relations before sending them.

I can understand you warning the user when the relation was ok and now it ends up empty. But for a newly created relation, I will never understand why you are asking when you should automatically delete it : there was no relation before the changeset, there will be no relation after the changeset and if the user wanted some, he could create them in a new changeset, no need to have empty relations at the first changeset.

Empty relations are ALWAYS bad datas, please improve JOSM to prevent to create them.

Best regards

[GerdP]

While I agree that empty relations are useless I really don't understand why you care so much about them. Did you try to convince the API developers to refuse the upload of empty relations?

[Fred73000]

When I opened this ticket, the title was "empty relations".
5 days later, someone changed it to "[PATCH] Delete relations by default when all members are deleted".
Since no more change so I think that everybody (including me) is ok with this title.
This ticket is now closed but the problem is not solved because your patch doesn't delete by default empty relations.

So the question is not why do I think it is not solved but why do you think it is solved.

[GerdP]

See #23527 for a regression.