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

Don't import "historic" WOF neighbourhoods #59

Closed
nvkelso opened this issue Dec 23, 2015 · 11 comments
Closed

Don't import "historic" WOF neighbourhoods #59

nvkelso opened this issue Dec 23, 2015 · 11 comments
Assignees
Milestone

Comments

@nvkelso
Copy link
Member

nvkelso commented Dec 23, 2015

When a WOF record has either the edtf:cessation or the edtf:deprecated, we should treat it the same as a record that has been superseded_by and not import it for tile generation (and if it's been imported in the past, we should remove it).

@nvkelso nvkelso added this to the v0.7.0 milestone Dec 23, 2015
@nvkelso
Copy link
Member Author

nvkelso commented Dec 23, 2015

/cc @thisisaaronland

@nvkelso nvkelso added the ready label Dec 23, 2015
@nvkelso
Copy link
Member Author

nvkelso commented Dec 23, 2015

Turns out these aren't in unix seconds but in YYYY-MM-dd formatted strings. So technically we should look at the dates and evaluate if they are in the future, and if they are, still allow them in today. But if they are today or yesterday or yesterday yesterday then drop them.

See also:

@zerebubuth zerebubuth added in review and removed ready labels Jan 7, 2016
@zerebubuth zerebubuth self-assigned this Jan 7, 2016
@zerebubuth
Copy link
Member

I had a crack at this and it seems it's harder than I expected. Basically, we've assumed in the code (all the way through, not just the WOF import code) that visibility of a feature is a function of its parameters and geometry only. Supporting the inception, cessation and deprecated parameters means that the visibility of a feature is a function of the current time as well.

This isn't a show-stopper, just means that we have to carry through the start and end dates from all neighbourhoods through to the database (which might involve some interesting ways of encoding "year infinity" in the database) and index them and have the tile update tool be more clever about what tiles it considers for updates (it needs to have an idea of the "last time I ran").

Feel free to move it around in the priority list if this change in effort level changes the priority.

@nvkelso
Copy link
Member Author

nvkelso commented Jan 13, 2016

LGTM, let's try it on dev.

@nvkelso
Copy link
Member Author

nvkelso commented Jan 21, 2016

Hmm, I'm still seeing the bunk United States neighbourhood label in Mesa, Arizona:

https://whosonfirst.mapzen.com/spelunker/id/85865451/
/#15/33.3108/-111.6676

screen shot 2016-01-20 at 16 33 05

@zerebubuth
Copy link
Member

Yeah, needed an update to the mapnik server as well. Once that was done, I forcibly refreshed that one tile: 15/6219/13165 (got tired of waiting for it in the humongous dev queue), and now it's gone. It's possible that there are others, but these should be caught when they're next updated.

@nvkelso
Copy link
Member Author

nvkelso commented Jan 21, 2016

I still see it in this tiles (and others)?

Caught when they're next updates meaning any of that deprecated records other fields are updated or? That may be a while... does this need a DB migration that effectively reloads all the neighbourhoods?

@zerebubuth
Copy link
Member

That tile might be cached already. The update that I did was pretty hacky, perhaps it can only be seen in the original json: http://vector.dev.mapzen.com/osm/all/15/6219/13165.json

We could do a mass-update, but there are so few neighbourhoods which have an inception / cessation that it's probably best to target some sort of one-off update for those.

@nvkelso
Copy link
Member Author

nvkelso commented Jan 21, 2016

Testing this with a new one that's marked deprecated today ("edtf:deprecated":"2016-01-21"):

  • data/858/288/13/85828813.geojson

Which is Korblex in Arcata, California:

screen shot 2016-01-21 at 10 39 32

@nvkelso
Copy link
Member Author

nvkelso commented Jan 21, 2016

Worked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants