Fix for #1077, fail to search `Saint..` cities in structured queries #1080

Joxit · 2018-01-23T15:52:49Z

orangejulius · 2018-01-29T19:21:48Z

Hey @Joxit,
Thanks for this fix. I haven't had the time to test it yet, but I suspect it will break handling of other Saint abbreviations, since this PR effectively reverses the logic from #767.

If you have a chance, can you test how some of the cases listed here work after your change? If not, we'll hopefully be able to get to it within the next couple weeks.

Joxit · 2018-01-30T11:15:09Z

Hi,
This PR does not change search queries, but has differents results for structured requests.

Example with Saint Marys, PA

Before:

GET v1/search/structured?locality=Saint Marys&region=PA HTTP/1.1

HTTP/1.1 200 OK
Content-Type: application/json; charset=utf-8

{
    "bbox": [
        -78.64852, 
        41.390487, 
        -78.419566, 
        41.523252
    ], 
    "features": [
        {
            "bbox": [
                -78.64852, 
                41.390487, 
                -78.419566, 
                41.523252
            ], 
            "geometry": {
                "coordinates": [
                    -78.534187, 
                    41.459777
                ], 
                "type": "Point"
            }, 
            "properties": {
                "accuracy": "centroid", 
                "confidence": 1, 
                "country": "United States", 
                "country_a": "USA", 
                "country_gid": "whosonfirst:country:85633793", 
                "county": "Elk County", 
                "county_gid": "whosonfirst:county:102081265", 
                "gid": "whosonfirst:locality:101716717", 
                "id": "101716717", 
                "label": "St. Marys, PA, USA", 
                "layer": "locality", 
                "localadmin": "St. Marys", 
                "localadmin_gid": "whosonfirst:localadmin:404486717", 
                "locality": "St. Marys", 
                "locality_gid": "whosonfirst:locality:101716717", 
                "match_type": "exact", 
                "name": "St. Marys", 
                "region": "Pennsylvania", 
                "region_a": "PA", 
                "region_gid": "whosonfirst:region:85688481", 
                "source": "whosonfirst", 
                "source_id": "101716717"
            }, 
            "type": "Feature"
        }
    ], 
    "geocoding": {
        "attribution": "http://places.jawg.io/v1/attribution", 
        "engine": {
            "author": "Mapzen", 
            "name": "Pelias", 
            "version": "1.0"
        }, 
        "query": {
            "lang": {
                "defaulted": true, 
                "iso6391": "en", 
                "iso6393": "eng", 
                "name": "English"
            }, 
            "parsed_text": {
                "city": "st marys", 
                "state": "PA"
            }, 
            "private": false, 
            "querySize": 20, 
            "size": 10
        }, 
        "timestamp": 1517309617785, 
        "version": "0.2"
    }, 
    "type": "FeatureCollection"
}

After:

GET v1/search/structured?locality=Saint Marys&region=PA HTTP/1.1

HTTP/1.1 200 OK
Content-Type: application/json; charset=utf-8

{
    "bbox": [
        -78.56086, 
        40.04178, 
        -75.34074, 
        41.45747
    ], 
    "features": [
        {
            "geometry": {
                "coordinates": [
                    -78.56086, 
                    41.42784
                ], 
                "type": "Point"
            }, 
            "properties": {
                "accuracy": "centroid", 
                "confidence": 1, 
                "country": "United States", 
                "country_a": "USA", 
                "country_gid": "whosonfirst:country:85633793", 
                "county": "Elk County", 
                "county_gid": "whosonfirst:county:102081265", 
                "gid": "geonames:locality:5210117", 
                "id": "5210117", 
                "label": "Saint Marys, PA, USA", 
                "layer": "locality", 
                "localadmin": "St. Marys", 
                "localadmin_gid": "whosonfirst:localadmin:404486717", 
                "locality": "Saint Marys", 
                "locality_gid": "geonames:locality:5210117", 
                "match_type": "exact", 
                "name": "Saint Marys", 
                "region": "Pennsylvania", 
                "region_a": "PA", 
                "region_gid": "whosonfirst:region:85688481", 
                "source": "geonames", 
                "source_id": "5210117"
            }, 
            "type": "Feature"
        }, 
        {
            "geometry": {
                "coordinates": [
                    -78.53418, 
                    41.45747
                ], 
                "type": "Point"
            }, 
            "properties": {
                "accuracy": "centroid", 
                "confidence": 1, 
                "country": "United States", 
                "country_a": "USA", 
                "country_gid": "whosonfirst:country:85633793", 
                "county": "Elk County", 
                "county_gid": "whosonfirst:county:102081265", 
                "gid": "geonames:locality:5210217", 
                "id": "5210217", 
                "label": "City of Saint Marys, PA, USA", 
                "layer": "locality", 
                "localadmin": "St. Marys", 
                "localadmin_gid": "whosonfirst:localadmin:404486717", 
                "locality": "City of Saint Marys", 
                "locality_gid": "geonames:locality:5210217", 
                "match_type": "exact", 
                "name": "City of Saint Marys", 
                "region": "Pennsylvania", 
                "region_a": "PA", 
                "region_gid": "whosonfirst:region:85688481", 
                "source": "geonames", 
                "source_id": "5210217"
            }, 
            "type": "Feature"
        }, 
        {
            "geometry": {
                "coordinates": [
                    -75.34074, 
                    40.04178
                ], 
                "type": "Point"
            }, 
            "properties": {
                "accuracy": "centroid", 
                "confidence": 1, 
                "country": "United States", 
                "country_a": "USA", 
                "country_gid": "whosonfirst:country:85633793", 
                "county": "Delaware County", 
                "county_gid": "whosonfirst:county:102080913", 
                "gid": "geonames:locality:5210213", 
                "id": "5210213", 
                "label": "Saint Marys Seminary, PA, USA", 
                "layer": "locality", 
                "localadmin": "Radnor", 
                "localadmin_gid": "whosonfirst:localadmin:404486571", 
                "locality": "Saint Marys Seminary", 
                "locality_gid": "geonames:locality:5210213", 
                "match_type": "exact", 
                "name": "Saint Marys Seminary", 
                "region": "Pennsylvania", 
                "region_a": "PA", 
                "region_gid": "whosonfirst:region:85688481", 
                "source": "geonames", 
                "source_id": "5210213"
            }, 
            "type": "Feature"
        }
    ], 
    "geocoding": {
        "attribution": "http://places.jawg.io/v1/attribution", 
        "engine": {
            "author": "Mapzen", 
            "name": "Pelias", 
            "version": "1.0"
        }, 
        "query": {
            "lang": {
                "defaulted": true, 
                "iso6391": "en", 
                "iso6393": "eng", 
                "name": "English"
            }, 
            "parsed_text": {
                "city": "saint marys", 
                "state": "PA"
            }, 
            "private": false, 
            "querySize": 20, 
            "size": 10
        }, 
        "timestamp": 1517309648187, 
        "version": "0.2"
    }, 
    "type": "FeatureCollection"
}

The query match GeoNames instead of Who's On First because in GN datas it's Saint-Marys and in WOF it's St. Marys... This simplification should perhaps be done in imports instead of API?

Joxit · 2018-02-28T13:23:02Z

This PR is linked to pelias/schema#268

With these changes, all these queries work properly :

/v1/search/structured?locality=St-Leu-la-Forêt&country=France and /v1/search/structured?locality=Saint-Leu-la-Forêt&country=France Better results than pelias-api master branch
/v1/search/structured?locality=Ste-Rose&region=Reunion and /v1/search/structured?locality=Sainte-Rose&region=Reunion
/v1/search/structured?locality=St-Remy-Les-Chevreuse&country=France and /v1/search/structured?locality=Saint-Remy-Les-Chevreuse&country=France Better results than pelias-api master branch
/v1/search/structured?locality=St-Pavace&country=France and /v1/search/structured?locality=Saint-Pavace&country=France Better results than pelias-api master branch
/v1/search/structured?locality=St Marys&region=PA and /v1/search/structured?locality=Saint Marys&region=PA
/v1/search/structured?locality=St Petersburg&region=FL and /v1/search/structured?locality=Saint Petersburg&region=FL
/v1/search/structured?locality=Sault Ste Marie&region=MI and /v1/search/structured?locality=Sault Sainte Marie&region=MI

orangejulius · 2018-03-01T04:27:30Z

It's great to see all those improvements. This change doesn't result in any regressions in our acceptance test suite, so it looks good to merge!

As for your earlier comment about how this could ideally be handled in WOF, I agree, so I looked into things. Most of these records already have the abbreviations expanded as alternate names. For example St Mary's PA:

That means Placeholder is already aware of these alternate names. However, the structured endpoint doesn't user Placeholder yet. So, this is a great short term fix. In the long term, Placeholder can help fix this issue and many more like it.

Thanks for all the work that went into this :)

orangejulius · 2018-03-01T04:42:16Z

Hi again,
In an effort to make this merge cleanly I have actually reverted these changes in YOUR master branch. Yes, github allows this. Hopefully this doesn't cause any issues, and my apologies if it does. Anyways, our master branch is up to date with these changes, so feel free to sync back up. Thanks again for the PR :)

Joxit · 2018-03-03T12:48:36Z

Hi, thanks :)
You are welcome, I will update my branch too ;)

ghost added the in review label Jan 23, 2018

orangejulius approved these changes Mar 1, 2018

View reviewed changes

ghost assigned orangejulius Mar 1, 2018

ghost added the in progress label Mar 1, 2018

orangejulius closed this Mar 1, 2018

orangejulius force-pushed the master branch from 1e50959 to c56336d Compare March 1, 2018 04:35

ghost added in progress and removed in progress labels Mar 1, 2018

orangejulius mentioned this pull request Mar 1, 2018

Improved Saint/Sainte synonym handling #1091

Merged

orangejulius mentioned this pull request Aug 16, 2018

Searches for streets with Saint or St in the name can fail when hitting the address_parts.street field pelias/pelias#737

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix for #1077, fail to search `Saint..` cities in structured queries #1080

Fix for #1077, fail to search `Saint..` cities in structured queries #1080

Joxit commented Jan 23, 2018 •

edited by orangejulius

Loading

orangejulius commented Jan 29, 2018

Joxit commented Jan 30, 2018 •

edited by orangejulius

Loading

Joxit commented Feb 28, 2018

orangejulius commented Mar 1, 2018 •

edited

Loading

orangejulius commented Mar 1, 2018

Joxit commented Mar 3, 2018

Fix for #1077, fail to search Saint.. cities in structured queries #1080

Fix for #1077, fail to search Saint.. cities in structured queries #1080

Conversation

Joxit commented Jan 23, 2018 • edited by orangejulius Loading

orangejulius commented Jan 29, 2018

Joxit commented Jan 30, 2018 • edited by orangejulius Loading

Joxit commented Feb 28, 2018

orangejulius commented Mar 1, 2018 • edited Loading

orangejulius commented Mar 1, 2018

Joxit commented Mar 3, 2018

Fix for #1077, fail to search `Saint..` cities in structured queries #1080

Fix for #1077, fail to search `Saint..` cities in structured queries #1080

Joxit commented Jan 23, 2018 •

edited by orangejulius

Loading

Joxit commented Jan 30, 2018 •

edited by orangejulius

Loading

orangejulius commented Mar 1, 2018 •

edited

Loading