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

Reinstate alternate names for aggregation types? #133

Closed
smmaurer opened this issue Apr 8, 2020 · 1 comment
Closed

Reinstate alternate names for aggregation types? #133

smmaurer opened this issue Apr 8, 2020 · 1 comment

Comments

@smmaurer
Copy link
Member

smmaurer commented Apr 8, 2020

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
Copy link
Member Author

smmaurer commented Aug 7, 2020

Fixed in PR #140 / v0.5

@smmaurer smmaurer closed this as completed Aug 7, 2020
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