Update road sort key values

[rmarianski]

Connects to tilezen/vector-datasource#546

@nvkelso could you review please?

[rmarianski]

I added some tests for this that might be easier to see how the values got affected: tilezen/tilequeue#74

I know it's not the greatest place, but I put it in tilequeue because we already had an easy way for the tests to automatically run.

[nvkelso]

Should also include the following Natural Earth values:
or type == 'Unknown'

[nvkelso]

Attached is my updated OpenOffice doc (look in column c, sort_key_v2):

sort_key_v2.ods.zip

[rmarianski]

@nvkelso I think I managed to incorporate all your feedback. The tests in the corresponding tilequeue pr have been updated too. You can probably more easily see what the new values for certain road types are there.

The tunnel values look to be too small now though. If the spreadsheet is the source of truth, it might be easier to try and transition the code to look more like a straight lookup, rather than a series of relative updates as it stands now.

[zerebubuth]

If the spreadsheet is the source of truth, it might be easier to try and transition the code to look more like a straight lookup, rather than a series of relative updates as it stands now.

Yes, please! The spreadsheet is almost detailed enough that code could be generated from it. As we bring more "style" into the server, perhaps it's worth considering how to make it easier to edit. All I can think of at the moment is generalising this to a post-process step which would take a series of columns and a CSV file as configuration parameters, the CSV file having the same columns in the same order plus one for the output.

[nvkelso]

Yes, the spreadsheet should probably be the source of truth (it certainly is for the landuse values). What more should I change in the spreadsheet so it's detailed enough? Probably split out the "reserved" ranges so there's as many reserved values (rows) instead?

[nvkelso]

55

[nvkelso]

With railway: 'spur', 'siding':
361 which is a -19

[nvkelso]

With railway: 'yard'
359, which is a -21

[nvkelso]

With railway other:
357 which is a -23

[nvkelso]

58

[nvkelso]

@rmarianski the tunnel values assume a start of 355 (not 335)?

[zerebubuth]

Yes, the spreadsheet should probably be the source of truth (it certainly is for the landuse values). What more should I change in the spreadsheet so it's detailed enough?

We could have it behave something like this. For example, if the config was:

  - fn: TileStache.Goodies.VecTiles.transform.spreadsheet
    params:
      source_layer: roads
      columns: [layer, is_tunnel, is_bridge, highway, railway, service]
      output_property: sort_key
      csv: csv/roads.csv

And roads.csv was:

layer, is_tunnel, is_bridge, highway, railway, service, sort_key
-3,*,*,*,*,*,305
-2,*,*,*,*,*,306
-1,*,*,*,*,*,307
1,*,*,*,*,*,378
2,*,*,*,*,*,379
3,*,*,*,*,*,380
*,yes,*,*,-,-,308
*,yes,*,-,*,*,309
*,yes,*,*,-,+,310
...

The way to interpret that would be that a feature would get the sort_key of the first row which matched the feature's properties, where:

1. A value matches only that value,
2. A (something)-delimited list matches any of those values,
3. A * matches anything, including a missing property,
4. A + matches anything except a missing property,
5. A - matches only a missing property.

However, a simpler approach might just be to port the Tangram style language filters, so the whole thing can be written in YAML?

@rmarianski what do you think?

[nvkelso]

Interesting, but that's sounding like new work we should undertake associated with giving every layer object (across layers) a sort_key?

[zerebubuth]

Interesting, but that's sounding like new work we should undertake associated with giving every layer object (across layers) a sort_key?

Sure. But I think it might be considerably less error-prone than trying to manually translate spreadsheets into code. And it'll mean considerably less reviewing to try and match up the spreadsheet to the code changes.

[rmarianski]

@zerebubuth I like this idea too. With this rule based approach though, would we need to worry about performance? We're talking about sequential rule matching for each feature right? This might not actually be a problem though, because we'll have about what, a few dozen rules? I especially like this if we can apply it as a framework for sort keys across all layers.

@nvkelso I incorporated your latest updates. Something I noticed is that now a railway with an unknown service is at the same exact level as a default road. Is that ok?

[nvkelso]

I especially like this if we can apply it as a framework for sort keys across all layers.

+1 for across all layers.

@rmarianski I thought it would be more like 2 above default road layer (357)?

* road layer default (paths, steps, tracks, etc)    355
* other road service (alley, driveway)  356
* other railway service 357
* unclassified, service, minor  358
* railway yard  359
* residential, unclassified, road, living_street, Unknown   360
* railway spur, siding  361

[rmarianski]

The case is a railway, where highway=service, and the service tag is set to something other than spur, siding, or yard.

https://github.com/mapzen/tilequeue/blob/a2881490d01e52cb02533aca8ddc4e1f3b6963a7/tests/test_transform.py#L35-L42

This might be the code that is unexpectedly dropping it down two values lower:

https://github.com/mapzen/TileStache/blob/102281336a95d7ee45dba6182df3441525265179/TileStache/Goodies/VecTiles/transform.py#L347-L349

[zerebubuth]

With this rule based approach though, would we need to worry about performance? We're talking about sequential rule matching for each feature right? This might not actually be a problem though, because we'll have about what, a few dozen rules?

Looks like about 223 rules from @nvkelso's original spreadsheet - about 73 for the roads layer.

It might be slow, but I don't know if it'll be that slow compared to other things we're doing (e.g: exterior boundaries, intercutting). If it is, then we'll have to look at other options, e.g: building a decision tree, JITting, byte code compiling.

I think the maintenance win makes it worthwhile, even if it turns out to be slightly slower.

[zerebubuth]