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

map GeoNames feature codes to the Pelias categories taxonomy (#20) #22

Merged
merged 13 commits into from
May 12, 2015

Conversation

sevko
Copy link
Contributor

@sevko sevko commented May 7, 2015

Map the extensive geonames feature codes onto the Pelias taxonomy.

  • add a category mappings file, metadata/category_mapping.json (mostly uses the same keys as openstreetmap, but there are some new ones like admin, industry, academic, natural:water)
  • set Document categories in task/import.js

fixes #20

sevko added 5 commits May 6, 2015 16:21
taxonomy.yaml
	-Add a first iteration of the geonames-to-pelias taxonomy
	mapping.
	-Currently a YAML file for human-writable convenience, will be
	converted to JSON later on.
	-Needs another look-over to revise some mappings/fill in missing
	ones.
taxonomy.yaml
	-Perform a second pass over the GeoNames taxonomy, tweaking some
	mappings added in 4f47082 and filling in empty ones. Roughly
	done, now.
taxonomy.yaml
	-Remove any GeoNames categories without corresponding categories
	in the Pelias taxonomy.
taxonomy.yaml -> metadata/taxonomy_mapping.json
	-YAML was initially used because it's much easier to handwrite
	than JSON. Now that the taxonomy mapping is complete, convert it
	to JSON.
"taxonomy" is less clear than "category".
task/import.js
	-Use the `category_mapping.json` file added in 4663087 to map
	geonames records' feature-codes to Pelias categories, and set
	them on `Document` objects.
@sevko sevko force-pushed the feature-taxonomy#20 branch from 62121a8 to aa03387 Compare May 7, 2015 18:02
sevko added a commit to pelias/acceptance-tests that referenced this pull request May 8, 2015
test_cases/categories.json
	-Add a bunch of test-cases for the addition of categories to
	GeoNames data (see [0]).
	-They aren't actually usable until the API can return
	categories, so must wait until [1] gets merged.

0: pelias/geonames#22
1: pelias/api#122
@missinglink
Copy link
Member

what do you see as the semantic difference between academic and education?
in the osm mapper natural:water maps to recreation https://github.com/pelias/openstreetmap/blob/master/config/category_map.js#L270

let's discuss further on a unified strategy.

@sevko
Copy link
Contributor Author

sevko commented May 11, 2015

Looks like I missed education when looking that file over, my fault. academic sounds like a better fit since it's more general (ie, a lab might be classified as academic but not necessarily educational), but seeing as it's already used in OSM I'm fine with leaving it as is, and changing all these occurrences of academic to education.

I departed from the OSM schema in some places, introducing categories like natural and natural:water, because what we currently have is a bit restrictive. It makes more sense to categorize a marsh, for instance, as simply a natural, watery area rather than a recreational one. I also added tags for industry, industry:agriculture, etc.

@missinglink
Copy link
Member

The idea is to provide a unified taxonomy, so things like https://github.com/pelias/openstreetmap/blob/master/config/category_map.js#L264 need to both map to the same pelias category.

In order to be consistent we need to either change the pelias/openstreetmap categories if they are wrong or change these ones if they are wrong, but we cannot have the same concept mapping to different categories.

@sevko
Copy link
Contributor Author

sevko commented May 11, 2015

Totally agree, we need to discuss the new categories before this gets merged.

@sevko
Copy link
Contributor Author

sevko commented May 11, 2015

As per these changes to the taxonomy, we've decided to add top-level keys industry and natural. We'll update the pelias/openstreetmap mapping accordingly. There are also several issues in this PR that I'll need to address:

  • there's a mix of agriculture and industry:agriculture categories; only use industry:agriculture
  • use the finance category where it makes sense
  • use education instead of academic

sevko added 4 commits May 11, 2015 11:35
metadata/category_mapping.json
	-`agriculture` was mistakenly used as a top-level category in
	some places; as per
	#22 (comment),
	replace all such occurrences with `industry:agriculture`.
metadata/category_mapping.json
	-Some of the Geonames categories also fall under the category of
	`finance`.
metadata/category_mapping.json
	-As per
	#22 (comment),
	use `education` instead of `academic`.
metadata/category_mapping.json
	-Some category sets with the `industry:agriculture` category
	were missing the superset `industry` category. Add it.
@missinglink
Copy link
Member

👍

sevko added 2 commits May 11, 2015 11:56
metadata/category_mapping.json
	-Typo in a category... oops.
@hkrishna
Copy link
Contributor

looks good 👍 :shipit:

peliasCategories.forEach( function ( category ){
try {
record.addCategory( category );
} catch ( ex ) {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably log this error

task/import.js
	-Log `addCategory()` errors.
	-Use `hasOwnProperty()` for greater readability.
	-cc @dianashk
@dianashk
Copy link
Contributor

👍

@sevko sevko merged commit eb1f980 into master May 12, 2015
@sevko sevko removed the in progress label May 12, 2015
@orangejulius orangejulius deleted the feature-taxonomy#20 branch February 5, 2016 15:56
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.

map FeatureClass and FeatureCode to the pelias taxonomy
4 participants