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

3387 - Add option to diff conflate to keep reviews in output and more #3588

Merged
merged 41 commits into from
Oct 31, 2019

Conversation

bwitham
Copy link
Contributor

@bwitham bwitham commented Oct 31, 2019

  • Added an option, differential.treat.reviews.as.matches, that allows for not treating reviews as matches in Differential Conflation, which will let them pass to the diff output. The use case for this was a situation where a one to many POI review was preventing several secondary POIs from being added to the diff. Since some of these reviews were questionable, the longer term solution to this is some reworking of POI reviews (Improve the POI reviews from #3387 #3579). In the meantime, this config option may be useful.
  • Disabled the removal/replacement of roundabouts during Differential Conflation. in Diff conflate roundabout mangling #3580 there was a situation where roundabouts were being mangled badly during roundabout replacement. The original point of roundabout removal/replacement was to preserve them, since we don't conflate them very well some of the time. However with diff conflate, there's no chance to mangle them since we're only keeping non-matches in the secondary data and not actually trying to merge ref and secondary roundabouts...so removing the logic was the simplest solution.
  • Downgraded warning logged when diff conflate w/ tags encounters a relation to a debug statement, since that is normal operation for now. Can/should we handle relations in diff conflate w/ tags? #3449 can be re-opened to handle relations if need be.
  • Fixed a NPE in the Network alg when running ref conflate with the same data from Add option to diff conflate to not treat reviews as matches #3387
  • Fixed issue where diff conflate case tests were not actually running diff conflate
  • Fxied handling of output size limit in MapComparator::_printIdDiff
  • Added in some utilities to Roundabout, OsmUtils, Node, and Way that make debugging roundabout conflate problems easier

@bwitham bwitham self-assigned this Oct 31, 2019
@bwitham bwitham changed the title 3387 - Add option to diff conflate to keep reviews in output 3387 - Add option to diff conflate to keep reviews in output and more Oct 31, 2019
@bwitham
Copy link
Contributor Author

bwitham commented Oct 31, 2019

@curranMapper This change implements 2) from our discussion in #3387 about POIs being dropped during diff conflate. There's now an option, "Treat reviews as matches and remove from output", under the diff options in the UI (turned off by default) which will allow reviews to pass through to the diff output if desired.

I opened #3579 as a longer term solution to make the POIs revews better.

@bwitham bwitham merged commit dc87c60 into master Oct 31, 2019
@bwitham bwitham deleted the 3387 branch October 31, 2019 16:36
@curranMapper
Copy link

Thanks @bwitham will try this out in testing.

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

Successfully merging this pull request may close these issues.

None yet

2 participants