Refactor collector

[adamfranco]

This is still a work in progress, but this branch includes all of the improvements to fix #20.

[Fonsan]

Great that sized down the diff from 4k lines to only 2k 😄

[Fonsan]

We should mention for devs that PYTHONPATH=lib py.test is how you run the tests

[Fonsan]

I found a problem with the current output format of the collector:

[{'coords': [(-73.00009, 43.70009),
              (-73.0001, 43.7001),
              (-73.00011, 43.70011),
              (-73.00012, 43.70012)],
   'id': 10004,
   'refs': [9, 10, 11, 12],
   'tags': {'highway': 'unclassified', 'name': 'Road A'}},
  {'coords': [(-73.00012, 43.70012),
              (-73.00013, 43.70013),
              (-73.00014, 43.70014),
              (-73.00015, 43.70015)],
   'id': 10003,
   'refs': [12, 13, 14, 15],
   'tags': {'highway': 'unclassified', 'name': 'Road A'}},
  {'coords': [(-73.00015, 43.70015),
              (-73.00016, 43.70016),
              (-73.00017, 43.70017),
              (-73.00018, 43.70018)],
   'id': 10000,
   'refs': [15, 16, 17, 18],
   'tags': {'highway': 'unclassified', 'name': 'Road A'}},
  {'coords': [(-73.00018, 43.70018),
              (-73.00019, 43.70019),
              (-73.0002, 43.7002),
              (-73.00021, 43.70021)],
   'id': 10001,
   'refs': [18, 19, 20, 21],
   'tags': {'highway': 'unclassified', 'name': 'Road A'}},
  {'coords': [(-73.00021, 43.70021),
              (-73.00022, 43.70022),
              (-73.00023, 43.70023),
              (-73.00024, 43.70024)],
    'id': 10002,
    'refs': [21, 22, 23, 24],
    'tags': {'highway': 'unclassified', 'name': 'Road A'}}
]

It does not expose any metadata about the join,

• What method was used to join the ways, by tag['name'] or tag['ref'] or other
• What is the identifier of the collection, maybe tag['name'] or tag['ref']
• What common tags do all ways have in the collection
• If ways where connected by tag['name'] or tag['ref'] but could not be joined through our join_ways algorithm etc

My point is that [way1,way2] leaves us no room to give any additional information in the future without breaking the protocol.

{'ways': [way1,way2], 'foo':'bar'} or even {'ways': [{'foo':'bar','way': way1}, {'foo':'baz','way': way2}]} gives us a lot more flexibility I think the latter one might be to verbose since a way is already a hash and we could append attributes to the way instead.

This would require a tweak of every post processor, but I feel that we should do it now rather then later when we presumably have more consumers.

[Fonsan]

Previously there was also a curvature value for a collection now there are only values for the ways

[adamfranco]

Thanks for all of the feedback, Erik. Here's my summary of a remaining to-do list before this branch will be a full fix for #20:

• [x] Change data structure of collections to be a dict that can hold both metadata and the constituent ways. E.g.:

  { 'join_type': 'name',
   'join_data': 'Main Street',
   'ways': [{}, {}, ...]

• [x] Add support for rolling up length and curvature values to the collection dict (in addition to the ways).
• [x] Update tests to add the module search path.
• [x] Verify that ways aren't being dropped during the join process. I don't think they are, but this should be checked and a test-case added for this.
• [x] Add test for joining ways by ref in addition to the existing name test.
• [x] Review #32 Assume a callback interface for collector
• [x] Merge #34 Added three identical example chains with relative path patch.
• [x] Update the README to reflect the new usage.

[Fonsan]

I think the checklist contains what we need in order to merge

[Fonsan]

Updated my pipeline to https://github.com/adamfranco/curvature/pull/28/commits/29b5f64ae630eb7369cc1261176bc7079ce92cde Which is the latest commit and found something that changed:

{'ways': [{'refs': [1933015060, 4024680285], 'coords': [[-17.618914199999992, 177.448986600001], [-17.61889380000007, 177.44915230000004]], 'id': 400191069, 'tags': {'highway': 'path'}}], 'join_type': 'none'}

This item has been run through AddCurvature etc but is now missing the curvature, most likely because of to few datapoints

[adamfranco]

@Fonsan, I've confirmed that ways aren't dropped during the join process and have added some comments to join_ways() to document this:

            # After we've either added all of the ways or looped the max_loop times,
            # add this collection to our collections list and return to the start
            # of our `while len(ways) > 0:` loop. The next remaining way in the
            # ways list will become the basis for a new collection that may get
            # some more of the remaining ways joined to it.

I've also added a unit test to verify. It took me a while staring at this function to figure out why it works. ;-)