Creating more filter options #101

jonha1 · 2024-07-22T23:55:34Z

This makes it easier to filter places that are nearby.

Issue#82:
https://github.com/soundscape-community/soundscape/issues/82

testing to see if commit works

This reverts commit 5a74197.

debugging filters

need to check this and other filters

jonha1 · 2024-07-22T23:56:37Z

I feel like I am close but having trouble understanding why it isn't working. If I can get help with one of them, I can create other filters.

steinbro

Can you provide a little more detail about what isn't working, what errors you're running into, etc.? I left a few comments/questions based on what jumps out to me in your diffs.

steinbro · 2024-07-23T12:42:18Z

apps/ios/GuideDogs/Assets/Localization/en-US.lproj/Localizable.strings

+/* Open Street Map term. An ice cream shop is a place where ice cream is sold. */
+"osm.tag.ice_cream" = "amenity=ice_cream";
+
+/* Open Street Map term. A pub is a place selling beer and other alcoholic drinks; may also provide food or accommodation (UK). */
+"osm.tag.pub" = "amenity=pub";


Don't include amenity= here. These should be the tag values, not keys.

steinbro · 2024-07-23T12:48:44Z

...s/GuideDogs/Code/Data/Models/Extensions/OSM Entity/GDASpatialDataResultEntity+Typeable.swift

+        let isFoodLocation = category == .places &&
+        localizedName.lowercased().contains(GDLocalizedString("osm.tag.restaurant").lowercased())


I was suggesting matching the amenity attribute of the place, rather than its name. I haven't tested this, but I'm thinking all you'd have to do is something like:

let isFoodLocation = amenity == GDLocalizedString("osm.tag.restaurant")

where amenity should be recognized as a field of GDASpatialDataResultEntity per

soundscape/apps/ios/GuideDogs/Code/Data/Models/Cache Models/GDASpatialDataResultEntity.swift

Line 47 in efcfa26

@objc dynamic var amenity: String!

steinbro · 2024-07-23T12:59:12Z

apps/ios/GuideDogs/Code/Data/Models/JSON Parsing/OSM/GeoJsonFeature.swift

+        if value == "fast_food"{
+            superCategory = SuperCategory.foods
+            return
+        }


Did this not work to assign the supercategory? This feels like the right place to be doing the classification of GeoJSON features, so the later filtering logic can be based entirely on the superCategory field.

I still think you can do the category assignment here. When the OSM feature is tagged with e.g. leisure=playground, the local variable type will be leisure here, and value will be playground.

jonha1 · 2024-07-24T23:50:39Z

Your code helped me gain a better understanding of it. Food saw more results however not all of them appeared. Landmarks seems to be working mostly well, some results dont match since I dont consdier them to be a landmark. For exmaple, the focus lab in Troy, NY isn't a landmark and I dont know why it is being classified as one. What should I do about it?

steinbro · 2024-07-25T12:39:46Z

For exmaple, the focus lab in Troy, NY isn't a landmark and I dont know why it is being classified as one. What should I do about it?

Here are the OSM tags for the FOCUS Lab: https://www.openstreetmap.org/node/10565967149

Of note is the tourism=gallery tag.

I searched for "gallery" in the Soundscape codebase, and found it here, being mapped to "landmark":

soundscape/apps/ios/GuideDogs/Assets/Default Super Categories/categories.json

Line 7 in efcfa26

    
           "landmark": ["waterfall", "boatyard", "theme_park", "roundhouse", "generator", "beach", "naval_base", "works", "water_works", "telescope", "pier", "observatory", "reservoir", "monument", "battlefield", "post_office", "planetarium", "social_centre", "prison", "courthouse", "bridge", "hangar", "tower", "attraction", "zoo", "gallery", "artwork", "alpine_hut", "plant", "insurance", "airfield", "water_tower", "pumping_station", "hot_water_tank", "campanile", "sports_centre", "beach_resort", "village_green", "ship", "memorial", "synagogue", "mosque", "chapel", "cathedral", "train_terminal", "college", "arts_centre", "ranger_station", "hospital", "fountain", "track", "conference_centre", "viewpoint", "supermarket", "peak", "storage_tank", "lighthouse", "beacon", "park", "port", "archaeological_site", "train_station", "shrine", "church", "historic_monument", "generic_landmark", "tourism_museum", "register_office", "grave_yard", "school", "marketplace", "fire_station", "ruins", "weir", "museum", "mall", "volcano", "hot_spring", "glacier", "wastewater_plant", "offshore_platform", "gasometer", "water_park", "bandstand", "wreck", "pillory", "monastery", "locomotive", "fort", "services", "lifeguard_tower", "temple", "national_park", "heliport", "public_park", "department_store", "studio", "public_building", "place_of_worship", "clock", "casino", "ferry_terminal", "stadium", "dam", "dock", "geyser", "bay", "barracks", "windmill", "watermill", "communications_tower", "swimming_area", "slipway", "nature_reserve", "marina", "ice_rink", "manor", "city_gate", "castle", "aircraft", "digester", "sally_port", "aerodrome", "shopping_mall", "cinema", "rescue_station", "airport", "theatre", "library", "university", "townhall", "police", "embassy", "bus_station", "station"],

That code is part of the OSM data ingestion, specifically to add custom fields that Soundscape uses. I believe the mapping there is used to determine what sound effect should be played for each map feature (for example, a "safety" item plays a different sound than a "landmark" does). The list includes many different types of locations, most of which wouldn't be considered landmarks in the colloquial sense, so we can't really depend on the category field for our purposes here.

- need to fix OSM tags and isPark

jonha1 · 2024-07-26T00:12:40Z

Thanks, the feedback greatly helped!

steinbro

Glad I could help. I assume you're still working on the other categories, cleaning up print statements, and adding some categorization test cases? I've also added a few more comments.

steinbro · 2024-07-26T19:11:25Z

...s/GuideDogs/Code/Data/Models/Extensions/OSM Entity/GDASpatialDataResultEntity+Typeable.swift

+        let lowercasedAmenity = amenity.lowercased()
+
+        let isParkLocation = parkTags.contains(lowercasedAmenity)


You'd want to do exact comparison rather than substrings, no? Otherwise you wind up matching e.g. parking lots as parks.

steinbro · 2024-07-26T19:14:45Z

...s/GuideDogs/Code/Data/Models/Extensions/OSM Entity/GDASpatialDataResultEntity+Typeable.swift

+        let parkTags = [
+            GDLocalizedString("osm.tag.park"),
+            GDLocalizedString("osm.tag.garden"),
+            GDLocalizedString("osm.tag.green_space"),
+            GDLocalizedString("osm.tag.recreation_area"),
+            GDLocalizedString("osm.tag.playground"),
+            GDLocalizedString("osm.tag.nature_reserve"),
+            GDLocalizedString("osm.tag.botanical_garden"),
+            GDLocalizedString("osm.tag.public_garden"),
+            GDLocalizedString("osm.tag.field"),
+            GDLocalizedString("osm.tag.reserve")
+        ]


I believe OSM tags are always in English, so no need to localize each of these. (I know I said to use localized strings, but that was when you were comparing against place names, which are in the local language.)

jonha1 · 2024-07-29T01:04:52Z

For test location (Latitude: 42.732782, Longitude: -73.68809) when I filter by park, I get a result named "leisure=garden". Do you know why? Is isPark how you want the other functions to be like?

steinbro · 2024-07-29T04:55:58Z

For test location (Latitude: 42.732782, Longitude: -73.68809) when I filter by park, I get a result named "leisure=garden". Do you know why? Is isPark how you want the other functions to be like?

Maybe there's a nearby green area that's tagged in OSM as leisure=park but has no name? I thought that the code already handled unnamed features gracefully, like crosswalks. In cases like these, the filtered features should have human-friendly names like "Park", and this is where you'll want to use localized strings for the name. I believe the code that constructs the localized name for an OSM feature is here: https://github.com/soundscape-community/soundscape/blob/main/apps/ios/GuideDogs/Code/Data/Models/Extensions/OSM%20Entity/GDASpatialDataResult%2BExtensions.swift#L54

jonha1 · 2024-08-01T03:33:28Z

It turns out that the place name defined by localizedName is "leasure=garden". I tested the same logic for landmakrs and it works. I'll impelment it for the other functions soon.

jonha1 · 2024-08-02T00:08:19Z

The functions should now have similiar logic and all works. The reason why the localizedName is "leisure=garden" is due to the fact that the garden doesnt have a name on open street maps. There is no name associated with it. How would you like me to handle it?

steinbro

The quick answer to your question is that if you simply tweak your localized strings to human-readable form, you'll wind up with nice human-readable names even for unnamed OSM entities. More feedback in comments below.

steinbro · 2024-08-15T12:34:03Z

apps/ios/GuideDogs/Assets/Localization/en-US.lproj/Localizable.strings

+"osm.tag.park" = "leisure=park";
+
+//TODO JON
+"osm.tag.garden" = "leisure=garden";
+"osm.tag.playground" = "leisure=playground";
+"osm.tag.nature_reserve" = "leisure=nature_reserve";
+"osm.tag.botanical_garden" = "leisure=botanical_garden";
+"osm.tag.public_garden" = "leisure=public_garden";
+"osm.tag.field" = "leisure=field";
+"osm.tag.reserve" = "leisure=reserve";
+"osm.tag.green_space" = "leisure=green_space";
+"osm.tag.recreation_area" = "leisure=recreation_area";


These should be the human-friendly version of the text, e.g. "osm.tag.public_garden" = "Public Garden";. This is what gets displayed in the UI when them entity doesn't have a name.

steinbro · 2024-08-15T12:35:33Z

apps/ios/GuideDogs/Code/Visual UI/Helpers/Nearby Table/NearbyTableFilter.swift

+                       self.image = UIImage(named: "Transit")
+                   case .food:
+                       self.localizedString = GDLocalizedString("filter.food_drink")
+                       self.image = UIImage(named: "Food and Drinks")


Do these icons exist? I see an image next to Transit, but not the others.

steinbro · 2024-08-15T12:36:14Z

apps/ios/GuideDogs/Code/Visual UI/Helpers/Nearby Table/NearbyTableFilter.swift

+                       self.localizedString = GDLocalizedString("filter.park")
+                       self.image = UIImage(named: "Parks")
+                   case .hotel:
+                       self.localizedString = GDLocalizedString("filter.hotel")


This key is missing from the English localizations.

steinbro · 2024-08-15T12:47:01Z

apps/ios/GuideDogs/Code/Data/Models/JSON Parsing/OSM/GeoJsonFeature.swift

+        if value == "fast_food"{
+            superCategory = SuperCategory.foods
+            return
+        }


I still think you can do the category assignment here. When the OSM feature is tagged with e.g. leisure=playground, the local variable type will be leisure here, and value will be playground.

steinbro · 2024-08-15T12:47:20Z

apps/ios/GuideDogs/Code/Visual UI/Helpers/Nearby Table/NearbyTableFilter.swift

+            NearbyTableFilter(type: .transit),
+            NearbyTableFilter(type: .food),
+            NearbyTableFilter(type: .landmarks),
+            NearbyTableFilter(type: .park),
+            NearbyTableFilter(type: .hotel)


We should be consistent with pluralization, both in variable names and displayed strings. Transit and Food are both collective nouns, but Landmarks, Parks, and Hotels need a trailing "s".

steinbro · 2024-08-15T12:50:37Z

...s/GuideDogs/Code/Data/Models/Extensions/OSM Entity/GDASpatialDataResultEntity+Typeable.swift

+            GDLocalizedString("osm.tag.restaurant"),
+            GDLocalizedString("osm.tag.fast_food"),
+            GDLocalizedString("osm.tag.cafe"),
+            GDLocalizedString("osm.tag.bar"),
+            GDLocalizedString("osm.tag.ice_cream"),
+            GDLocalizedString("osm.tag.pub"),
+            GDLocalizedString("osm.tag.coffee_shop")


You don't need to use localized strings here, because the OSM tag will always be e.g. "restaurant" regardless of what language the name of the restaurant is in. I think you're handling this properly in landmarks and parks below.

steinbro · 2024-09-07T21:43:54Z

@jonha1 I went ahead and cleaned this up since I wanted to see this deployed. I figured you wouldn't mind. :)

@RDMurray This is ready for inclusion in the next TestFlight build. I haven't merged since I wasn't sure whether you prefer we merge branches before or after they've been tested.

RDMurray · 2024-09-10T15:45:00Z

It looks good here, categories are working in my local area.

I don't have any objection to you merging PRs in the future when you think they are ready.

jonha1 and others added 11 commits July 11, 2024 21:53

test

5a74197

testing to see if commit works

Revert "test"

e507a88

This reverts commit 5a74197.

testing again

b9d0795

added tests and need to implement logic

72de1ae

changed name of filters on app

0784e5b

switch filtering to education

7c08f36

debuggign finding nearby restaurants

38f7713

addingn filter for landmakrs instead of education

2d72132

debugging filters

filter by restaurants and landmarks

aa738ee

testing parks nearby function

26f477e

need to check this and other filters

trying to create isFood filter

fc66606

steinbro reviewed Jul 23, 2024

View reviewed changes

fixing food and landmark

79a0629

fixed landmark and food

60f11c0

- need to fix OSM tags and isPark

steinbro reviewed Jul 26, 2024

View reviewed changes

cleanup

0cf1e84

parks sitll doenst work but landmakrs works now

4b83c94

debugging park

97e660e

steinbro reviewed Aug 15, 2024

View reviewed changes

Clean up Places Nearby filter implementation

3ffa533

Revert changes to AuthoredActivityLoader.swift

02d7c31

Fix NearbyTableFilterTest switch

fc6cb05

RDMurray merged commit 5f5ade4 into soundscape-community:main Sep 10, 2024
1 check passed

RDMurray mentioned this pull request Sep 10, 2024

Provide more filtering options of nearby places e.g., places to eat, parks, businesses etc #82

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Creating more filter options #101

Creating more filter options #101

jonha1 commented Jul 22, 2024

jonha1 commented Jul 22, 2024

steinbro left a comment

steinbro Jul 23, 2024

steinbro Jul 23, 2024

steinbro Jul 23, 2024

steinbro Aug 15, 2024

jonha1 commented Jul 24, 2024

steinbro commented Jul 25, 2024

jonha1 commented Jul 26, 2024

steinbro left a comment

steinbro Jul 26, 2024

steinbro Jul 26, 2024

jonha1 commented Jul 29, 2024

steinbro commented Jul 29, 2024

jonha1 commented Aug 1, 2024

jonha1 commented Aug 2, 2024 •

edited

Loading

steinbro left a comment

steinbro Aug 15, 2024

steinbro Aug 15, 2024

steinbro Aug 15, 2024

steinbro Aug 15, 2024

steinbro Aug 15, 2024

steinbro Aug 15, 2024

steinbro commented Sep 7, 2024

RDMurray commented Sep 10, 2024

		let isFoodLocation = category == .places &&
		localizedName.lowercased().contains(GDLocalizedString("osm.tag.restaurant").lowercased())

		let lowercasedAmenity = amenity.lowercased()

		let isParkLocation = parkTags.contains(lowercasedAmenity)

Creating more filter options #101

Creating more filter options #101

Conversation

jonha1 commented Jul 22, 2024

jonha1 commented Jul 22, 2024

steinbro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonha1 commented Jul 24, 2024

steinbro commented Jul 25, 2024

jonha1 commented Jul 26, 2024

steinbro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonha1 commented Jul 29, 2024

steinbro commented Jul 29, 2024

jonha1 commented Aug 1, 2024

jonha1 commented Aug 2, 2024 • edited Loading

steinbro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

steinbro commented Sep 7, 2024

RDMurray commented Sep 10, 2024

jonha1 commented Aug 2, 2024 •

edited

Loading