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

Add more placetypes to download #280

Merged
merged 4 commits into from
Sep 21, 2017
Merged

Conversation

trescube
Copy link
Contributor

Add empire, ocean, and marinearea to download. Also synthesizes a hierarchy from the id and placetype when no hierarchies are in the source data. This is useful for top-level placetypes like oceans that have no hierarchies. Without this, the ES document would not have ocean, ocean_id, and ocean_a with is inconsistent with lower level placetypes like neighbourhoods where the hierarchy ensures that neighbourhhood, neighbourhood_id, and neighbourhood_a are in the ES documents.

@ghost ghost assigned trescube Sep 13, 2017
@ghost ghost added the in review label Sep 13, 2017
Copy link
Member

@orangejulius orangejulius left a comment

Choose a reason for hiding this comment

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

looks good, but we want to pin to a particular version of pelias-model, right?

@@ -31,12 +31,15 @@ Currently, the supported hierarchy types are:
- county
- dependency
- disputed
- [empire](https://www.youtube.com/watch?v=-bzWSJG93P8)
Copy link
Member

Choose a reason for hiding this comment

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

lol

package.json Outdated
@@ -41,7 +41,7 @@
"pelias-config": "2.12.0",
"pelias-dbclient": "2.2.2",
"pelias-logger": "0.2.0",
"pelias-model": "5.0.1",
"pelias-model": "pelias/model",
Copy link
Member

Choose a reason for hiding this comment

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

whats this?

Copy link
Member

Choose a reason for hiding this comment

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

Oh you're probably waiting for this to be merged, right? pelias/model#75

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, I'm stalling on merging that for a bit as I worked out how this effects the whole ecosystem.

@trescube trescube force-pushed the add-more-placetypes-to-download branch 4 times, most recently from 23be0c5 to 06573ea Compare September 21, 2017 15:24
@trescube trescube force-pushed the add-more-placetypes-to-download branch from 06573ea to 0f9ea52 Compare September 21, 2017 17:54
@trescube trescube merged commit 7e83c8f into master Sep 21, 2017
@ghost ghost removed the in review label Sep 21, 2017
@trescube trescube deleted the add-more-placetypes-to-download branch September 21, 2017 18:28
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.

2 participants