ltog / osmi-addresses

Calculates the Address view of the OSM Inspector
Boost Software License 1.0
11 stars 4 forks source link

Search for also places that correspond to addr:street #115

Closed johsin18 closed 5 years ago

johsin18 commented 6 years ago

This is a PR to solve issue #111.

All commits but the last are cleanup and refactoring.

I tested only using osmi-testzone.osm, and I ran it once on the Swiss data set. I was not able to test the results graphically (probably involves some huge setup). Can somebody please test it graphically, e.g. the addresses around Tellplatz Basel should not be shown red anymore: http://tools.geofabrik.de/osmi/?view=addresses&lon=7.59483&lat=47.54331&zoom=17&overlays=buildings,buildings_with_addresses,postal_code,entrances_deprecated,entrances,street_not_found,interpolation,interpolation_errors,connection_lines,nearest_points,nearest_roads,nearest_areas,addrx_on_nonclosed_way

johsin18 commented 6 years ago

@ltog @Nakaner I would appreciate your comments.

ltog commented 6 years ago

@johsin18 (CC @Nakaner ): Thank you for taking the time to write the pull request and sorry for the delayed answer. I started working on a file for Vagrant to resolve your mentioned

I was not able to test the results graphically (probably involves some huge setup).

by providing a one-click solution to setup osmi-addresses. While preparing the Vagrantfile I discovered some issues with the GDAL version provided by Ubuntu 18.04 which I began to address here: https://github.com/ltog/osmi-addresses/commits/gdal_fix

I'll be at SotM next weekend. If you'll be there too, we could meet.

johsin18 commented 6 years ago

So finally I was able to set up MapServer and test my changes on the map. Following the README, it was less hard than expected. I though I would have to also set up a server serving the base tile layers, but that's not needed, fortunately.

So indeed my code was still buggy, but that should be fixed now.

I would appreciate if you would merge my branch.

ltog commented 6 years ago

@johsin18 : I'm glad the README was helpful...

Recently I met with @Nakaner . We weren't sure if tying together addr:street=* with place=square, name=* is something we should encourage through osmi-addresses. We therefore decided to ask for a broader opinion at the tagging mailinglist. You can find my message at https://lists.openstreetmap.org/pipermail/tagging/2018-August/038415.html . The whole structure of the discussion will be visible at https://lists.openstreetmap.org/pipermail/tagging/2018-August/thread.html once it took place.

Sorry, that this takes so long...

johsin18 commented 6 years ago

Well, you had almost one year time to think about this, and now you change your minds :-(

If you don't want this change, please merge at least my code cleanups, so that my work was not fully in vain.

Nakaner commented 5 years ago

Thank you for your contribution.

I decided to merge this pull request (not the refactoring only) because there are indeed cases where place=square looks unavoidable to me. The Tellplatz example in Basel is not such a case because it would be possible to name the pedestrian areas.

ltog commented 5 years ago

@Nakaner : Thank you for taking care of this!

Nakaner commented 5 years ago

The Mapserver styles have been deployed now.

johsin18 commented 5 years ago

Thanks for taking this in.