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

[editor] Add flag to changeset when features are merged #8186

Merged
merged 1 commit into from
May 16, 2024

Conversation

map-per
Copy link
Member

@map-per map-per commented May 15, 2024

Re-added flagging, like in #7853, but for "info:features_merged=yes" instead of "feature_cloes_by".
The "info:features_merged=yes" tag is added to changesets when the feature merging code is executed. This makes evaluating / making statistics for #2298 possible (see: #7853 (comment)).

✔️ Tested in https://www.openstreetmap.org/changeset/151378253

@map-per map-per changed the title [editor] Ad flag to changeset when features are merged [editor] Add flag to changeset when features are merged May 15, 2024
@map-per map-per requested a review from biodranik May 15, 2024 18:42
Copy link
Member

@biodranik biodranik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the value in the stats? I've checked the code, it takes any nearby feature in 1 meter radius to match. There is no any other "matching" logic implemented at all, even a simple tags comparison is missing )

It should be easy to implement it though.

@map-per
Copy link
Member Author

map-per commented May 16, 2024

I think the flagging makes sense. A merging algorithm that deciding whether to add or merge features is not straightforward / trivial (e.g. if users add the same shop, but with slightly different name or when there are two distinct shops on top of each other on different floors of a shopping centre). So even even when we finally have a good merging algorithm it will make mistakes, so the "info:objects_merged=yes" changesets are still especially interesting for human reviewers. (Without the flag I don't know any way of filtering for merge changeset with standard OSM reviewing software). And for developing the algorithm it is also helpful to not miss out edge cases or try out the algorithm on paper with real world example data. At the moment we don't know how often #2298 occurs and we don't know what common cases there are that a merging algorithm should handle correctly.

Copy link
Member

@biodranik biodranik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, very good arguments for debugging and improving the algo. Thanks for the PR!

@biodranik biodranik merged commit 60333f8 into organicmaps:master May 16, 2024
15 checks passed
@map-per map-per deleted the flagging branch May 17, 2024 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants