UDST / pandana

Pandas Network Analysis by UrbanSim: fast accessibility metrics and shortest paths, using contraction hierarchies :world_map:
http://udst.github.io/pandana
GNU Affero General Public License v3.0
387 stars 83 forks source link

Reinstate alternate names for aggregation types? #133

Closed smmaurer closed 4 years ago

smmaurer commented 4 years ago

Just discovered this about network.aggregate():

Pandana v0.3 and earlier supported alternate names for some of the aggregation types. The docstring specifies 'ave', 'std', and 'median', but the code itself supports some aliases as well: 'average' in place of 'ave', 'stddev' in place of 'std', and 'med' in place of 'median'.

v0.3.0/network.py#L17-L29
v0.3.0/network.py#L334

Beginning with v0.4.0, the code only supports the names from the docstring.

v0.4.0/network.py#L322
v0.4.0/accessibility.cpp#L310-L370

Some of the undocumented names are used in Bay Area UrbanSim, which led to a Pandas value error when Pandana collects the data to aggregate: ValueError: Length of passed values is 0, index implies 226060.

Solution

I'd propose reinstating the alternate aggregation names, and adding them to the documentation -- this will improve support for old code, and improve the ux.

We could also add support for 'mean' as an alias for 'ave', which is probably a common mistake people make.

And we should add some validation to catch cases where the user doesn't pass a valid aggregation type.

smmaurer commented 4 years ago

Fixed in PR #140 / v0.5