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

Merge street number hits with streets in result #429

Closed

Conversation

agunnarsson
Copy link

This change mixes street number hits with streets where the sought number isn't found. A remaining issue exists for streets with multiple segments, where both a street number hit and the remaining segments where the number isn't found are included in the response.

Fixes #422

@lonvia
Copy link
Member

lonvia commented May 1, 2016

Looks generally good. One thing I noticed is that deduplication might be confused now. If there are two ways in OSM with the same name and address and for one of them, the exact address could be found, then the other should normally be eliminated from the results as well unless deduplication has been disabled with parameter dedupe=0. Do you think that is possible?

@agunnarsson
Copy link
Author

If you refer to the case when one street has multiple segments (ways) in the data, I noticed that you get both a street number hit and all the segments except the one with the hit, which is indeed confusing.
Question is how to fix that. It might be possible traverse segments connected to the one with the hit, but then it will just work where all segments of a street are connected. Another solution would be to use address details and maybe coordinates to figure out which segments are part of the same street, but that may involve one or more new queries. I'll have a look at it.

@agunnarsson
Copy link
Author

The last commit should fix the issue with both street number and related ways in the response.

Maybe the query in the new function getRelated() should be a function in the database instead? It might be useful for other things, like street number interpolation. What do you think?

I have not been able to test on a full planet database other than running the new query, since our installation is too old.

@lonvia lonvia mentioned this pull request May 27, 2016
@lonvia
Copy link
Member

lonvia commented Jun 9, 2016

Apologies for taking so long to review. The interpolation PR was keeping me busy. Alas, it is in conflict with this PR, so this one needs to be rebased.

I really like your approach to deduplication but I think it comes a bit late in the sense that it goes back again to the database to do the deduplication. It would be much better to merge results already when doing the initial query over search_names. Needs some more thinking but should be doable.

For the deduplication problem at hand, I'd rather prefer an approach that works on the local results and simply extends the existing deduplication function around https://github.com/twain47/Nominatim/blob/master/lib/Geocode.php#L1875

@agunnarsson
Copy link
Author

No problem! I understand that you have other things to do, and maybe have a need for vacation once in a while. So there are no expectations of immediate responses.

It would be much better to merge results already when doing the initial query over search_names. Needs some more thinking but should be doable.

This would mean integrating the following street number queries directly in in the the initial query?

For the deduplication problem at hand, I'd rather prefer an approach that works on the local results and simply extends the existing deduplication function around https://github.com/twain47/Nominatim/blob/master/lib/Geocode.php#L1875

I'm not sure I follow the intention of the current deduplication. It seems to ensure that every osm feature is included once, and that there is only one combination of name and type. To me it seems that the latter part has potential to remove wanted results.
What would be a correct way to do deduplication related to this issue? Only remove street segments related to a street number if bDeDupe is true? Maybe also include only one segment per street where no street number is found? That would mean a need to keep information about related street segments from the initial query.

If we can iron out the details about how we want this implemented, I will hopefully be able to find some time to fix it before the upcoming summer vacations.

@lonvia
Copy link
Member

lonvia commented Apr 30, 2019

This is now basically implemented in 85f32d6.

@lonvia lonvia closed this Apr 30, 2019
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.

Missing street numbers in combination with common street names can give confusing results
2 participants