Skip to content
This repository has been archived by the owner. It is now read-only.

Only keep most granular location for a given name #133

Merged
merged 4 commits into from Sep 11, 2017

Conversation

Projects
None yet
2 participants
@c-w
Copy link
Member

commented Sep 11, 2017

For example, if we have a location name that is both a city and a state (like "New York") we only want to keep the city.

Implementation note: keeping the LocationExtractorFactory's lookup field as a Map[String, Set[Location]] although only a single location will be in the set after this change to avoid changing the public API of the factory (i.e., minimize risk surface).

@c-w c-w requested review from jcjimenez and erikschlegel Sep 11, 2017

@c-w c-w added the in progress label Sep 11, 2017

Only keep most granular location for a given name
For example, if we have a location name that is both a city and a state
(like "New York") we only want to keep the city.

@c-w c-w force-pushed the keep-only-granular-locations branch from f4b625d to 633ef09 Sep 11, 2017

@@ -107,7 +107,7 @@ object CassandraTest {
title = "twitter post" ),
analysis = Analysis(
sentiments = List(.6),
locations = List(Location(wofId = "wof-85670485", confidence = Option(1.0), latitude = 12.21, longitude = 43.1)),
locations = List(Location(wofId = "wof-85670485", layer = "coutry", confidence = Option(1.0), latitude = 12.21, longitude = 43.1)),

This comment has been minimized.

Copy link
@kevinhartman

kevinhartman Sep 11, 2017

Contributor

Typo in country

This comment has been minimized.

Copy link
@c-w

c-w Sep 11, 2017

Author Member
* @see https://github.com/whosonfirst/whosonfirst-placetypes
*/
def layerToInt(layer: String): Int = {
if ("continent".equalsIgnoreCase(layer)) {

This comment has been minimized.

Copy link
@kevinhartman

kevinhartman Sep 11, 2017

Contributor

Can you lowercase layer first and then do a match expression here?

This comment has been minimized.

Copy link
@c-w

c-w Sep 11, 2017

Author Member
logDebug(s"Discarding location ${oldLocation.wofId} for name $locationName as we now have more granular location ${newLocation.wofId}")
map(locationName) = newLocation
} else {
logDebug(s"Discarding location ${newLocation.wofId} for name $locationName since we already have more granular location ${oldLocation.wofId}")

This comment has been minimized.

Copy link
@kevinhartman

kevinhartman Sep 11, 2017

Contributor

"Ignoring" may be clearer for ignored incoming locations, especially at a quick glance while debugging (differentiating from a more specific location being found in the logs).

This comment has been minimized.

Copy link
@c-w

c-w Sep 11, 2017

Author Member

@c-w c-w merged commit cfc6736 into master Sep 11, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@c-w c-w deleted the keep-only-granular-locations branch Sep 11, 2017

@c-w c-w removed the in progress label Sep 11, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.