After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 725899 - Add more place types to GeocodePlace
Add more place types to GeocodePlace
Status: RESOLVED FIXED
Product: geocode-glib
Classification: Other
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: geocode-glib maintainer(s)
geocode-glib maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2014-03-07 13:57 UTC by Jonas Danielsson
Modified: 2014-03-27 18:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add amenity place types to GeocodePlace (28.68 KB, patch)
2014-03-07 13:58 UTC, Jonas Danielsson
none Details | Review
Add amenity place types to GeocodePlace (28.77 KB, patch)
2014-03-07 14:02 UTC, Jonas Danielsson
none Details | Review
Add amenity place types to GeocodePlace (4.44 KB, patch)
2014-03-07 14:19 UTC, Jonas Danielsson
none Details | Review
Make sure we use the airport type for GeocodePlace (1.47 KB, patch)
2014-03-07 14:19 UTC, Jonas Danielsson
needs-work Details | Review
Add amenity place types to GeocodePlace (28.55 KB, patch)
2014-03-26 13:03 UTC, Jonas Danielsson
none Details | Review
Add amenity place types to GeocodePlace (28.55 KB, patch)
2014-03-26 13:06 UTC, Jonas Danielsson
needs-work Details | Review
Add amenity place types to GeocodePlace (2.91 KB, patch)
2014-03-26 21:26 UTC, Jonas Danielsson
committed Details | Review
Add icons for new amenity place types (1.79 KB, patch)
2014-03-26 21:26 UTC, Jonas Danielsson
committed Details | Review
forward: Include airport place type in results (1011 bytes, patch)
2014-03-26 21:27 UTC, Jonas Danielsson
committed Details | Review
build: Add icon for airtport place type (791 bytes, patch)
2014-03-26 21:27 UTC, Jonas Danielsson
committed Details | Review

Description Jonas Danielsson 2014-03-07 13:57:30 UTC
To get a bit better granularity when searching we could add some more types.
I went through the amenity category, here: taginfo.openstreetmap.org/keys/amenity#values and choose a few that seemed appropriate.

Patch forthcoming.
Comment 1 Jonas Danielsson 2014-03-07 13:58:11 UTC
Created attachment 271229 [details] [review]
Add amenity place types to GeocodePlace

The place types selected here are among the top of the amenity
category.

The icons are, as before, taken from the Maki mapbox set.
Comment 2 Jonas Danielsson 2014-03-07 14:02:44 UTC
Created attachment 271232 [details] [review]
Add amenity place types to GeocodePlace

The place types selected here are among the top of the amenity
category from http://taginfo.openstreetmap.org/keys/amenity#values

The icons are, as before, taken from the Maki mapbox set.
Comment 3 Jonas Danielsson 2014-03-07 14:19:19 UTC
Hmm, I guess this patch should be split up with the fixing up of GEOCODE_PLACE_TYPE_AIRPORT as separate patch.
Comment 4 Jonas Danielsson 2014-03-07 14:19:52 UTC
Created attachment 271235 [details] [review]
Add amenity place types to GeocodePlace

The place types selected here are among the top of the amenity
category from http://taginfo.openstreetmap.org/keys/amenity#values

The icons are, as before, taken from the Maki mapbox set.
Comment 5 Jonas Danielsson 2014-03-07 14:19:56 UTC
Created attachment 271236 [details] [review]
Make sure we use the airport type for GeocodePlace
Comment 6 Jonas Danielsson 2014-03-26 13:03:30 UTC
Created attachment 272987 [details] [review]
Add amenity place types to GeocodePlace

The place types selected here are among the top of the amenity
category from http://taginfo.openstreetmap.org/keys/amenity#values

The icons are, as before, taken from the Maki mapbox set.
Comment 7 Jonas Danielsson 2014-03-26 13:06:01 UTC
Created attachment 272988 [details] [review]
Add amenity place types to GeocodePlace

The place types selected here are among the top of the amenity
category from http://taginfo.openstreetmap.org/keys/amenity#values

The icons are, as before, taken from the Maki mapbox set.
Comment 8 Zeeshan Ali 2014-03-26 17:25:24 UTC
Review of attachment 271236 [details] [review]:

"More specific place type for airports" would be more appropriate shortlog i think. :)

::: icons/Makefile.am
@@ +11,3 @@
 	scalable_places_poi-restaurant.svg \
+	scalable_places_poi-bar.svg \
+	scalable_places_poi-airport.svg

Should be seperate patch ideally but if not then commit log should reflect this.
Comment 9 Zeeshan Ali 2014-03-26 17:27:20 UTC
Review of attachment 272988 [details] [review]:

Looks good but i'd rather icons are added in separate patch.
Comment 10 Jonas Danielsson 2014-03-26 20:24:21 UTC
Review of attachment 271236 [details] [review]:

Hmm, ok. Before this patch PLACE_TYPE_AIRPORT wasn't used at all. Maybe "forward: Include PLACE_TYPE_AIRPORT in Nominatim conversion"?

::: icons/Makefile.am
@@ +11,3 @@
 	scalable_places_poi-restaurant.svg \
+	scalable_places_poi-bar.svg \
+	scalable_places_poi-airport.svg

I'll split it out.
Comment 11 Jonas Danielsson 2014-03-26 20:24:46 UTC
Review of attachment 272988 [details] [review]:

Thanks. Ok!
Comment 12 Jonas Danielsson 2014-03-26 21:26:49 UTC
Created attachment 273026 [details] [review]
Add amenity place types to GeocodePlace

The place types selected here are among the top of the amenity
category from http://taginfo.openstreetmap.org/keys/amenity#values
Comment 13 Jonas Danielsson 2014-03-26 21:26:58 UTC
Created attachment 273027 [details] [review]
Add icons for new amenity place types
Comment 14 Jonas Danielsson 2014-03-26 21:27:06 UTC
Created attachment 273028 [details] [review]
forward: Include airport place type in results
Comment 15 Jonas Danielsson 2014-03-26 21:27:11 UTC
Created attachment 273029 [details] [review]
build: Add icon for airtport place type
Comment 16 Zeeshan Ali 2014-03-26 23:31:26 UTC
Review of attachment 273026 [details] [review]:

ack
Comment 17 Zeeshan Ali 2014-03-26 23:31:56 UTC
Review of attachment 273027 [details] [review]:

ack
Comment 18 Zeeshan Ali 2014-03-26 23:32:59 UTC
Review of attachment 273028 [details] [review]:

ack
Comment 19 Zeeshan Ali 2014-03-26 23:33:50 UTC
Review of attachment 273029 [details] [review]:

Isn't the code to actually use the icon missing?
Comment 20 Jonas Danielsson 2014-03-27 05:46:45 UTC
Review of attachment 273029 [details] [review]:

It was already there, the place type in forward and the switch case in place already had AIRPORT support. What was missing was the icon and setting it from OSM attributes.
Comment 21 Zeeshan Ali 2014-03-27 17:31:32 UTC
Review of attachment 273029 [details] [review]:

Ah ok, then I guess this patch comes before the others (where its used?).
Comment 22 Jonas Danielsson 2014-03-27 18:30:49 UTC
(In reply to comment #21)
> Review of attachment 273029 [details] [review]:
> 
> Ah ok, then I guess this patch comes before the others (where its used?).

Yeah.
Comment 23 Jonas Danielsson 2014-03-27 18:34:39 UTC
Attachment 273026 [details] pushed as 9b0b84c - Add amenity place types to GeocodePlace
Attachment 273027 [details] pushed as abbba5b - Add icons for new amenity place types
Attachment 273028 [details] pushed as 7eb1490 - forward: Include airport place type in results
Attachment 273029 [details] pushed as 4d4d5b5 - build: Add icon for airtport place type