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

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

Merged
merged 13 commits into from Mar 25, 2019

Conversation

johsin18
Copy link
Contributor

@johsin18 johsin18 commented Jul 2, 2018

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

This is not specific to this code, developers should know how to use their tools.
Should be imported to Eclipse as a Makefile project, then all that should be implicit.
@johsin18 johsin18 changed the title Place square Search for also places that correspond to addr:street Jul 5, 2018
@johsin18
Copy link
Contributor Author

@ltog @Nakaner I would appreciate your comments.

@ltog
Copy link
Owner

ltog commented Jul 20, 2018

@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
Copy link
Contributor Author

johsin18 commented Aug 9, 2018

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
Copy link
Owner

ltog commented Aug 13, 2018

@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
Copy link
Contributor Author

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 Nakaner merged commit 5b0377a into ltog:master Mar 25, 2019
@Nakaner
Copy link
Collaborator

Nakaner commented Mar 25, 2019

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
Copy link
Owner

ltog commented Mar 25, 2019

@Nakaner : Thank you for taking care of this!

@Nakaner
Copy link
Collaborator

Nakaner commented Mar 29, 2019

The Mapserver styles have been deployed now.

@johsin18
Copy link
Contributor Author

Thanks for taking this in.

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

3 participants