search

pysal / momepy

Urban Morphology Measuring Toolkit
https://docs.momepy.org
BSD 3-Clause "New" or "Revised" License
496 stars 59 forks source link

ENH: describe as a replacement of AverageCharacter #570

Closed martinfleis closed 7 months ago

martinfleis commented 7 months ago

I started refactoring AverageCharacter and changed my mind a couple of times on what to do about it. In the end, decided to generalise it into momepy.describe which computes "mean", "median", "std", "min", "max", "sum" and optionally "mode" of values within the neighborhood, possibly after cutting the extremes off. We still have the full functionality of the original AverageCharacter folded in though.

I am still uncertain whether this is the right API, so I'd appreciate some input @jGaboardi and @u3ks.

Also, this may be more fitting in libpysal.graph but it may be wise to keep the API there a bit leaner.

Tests are missing as of now but will add them tomorrow.

codecov[bot] commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 97.8%. Comparing base (4037c70) to head (7c04fa0). Report is 15 commits behind head on main.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/pysal/momepy/pull/570/graphs/tree.svg?width=650&height=150&src=pr&token=VNn0WR5JWT&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal)](https://app.codecov.io/gh/pysal/momepy/pull/570?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal) ```diff @@ Coverage Diff @@ ## main #570 +/- ## ======================================= + Coverage 97.4% 97.8% +0.4% ======================================= Files 26 35 +9 Lines 4328 5078 +750 ======================================= + Hits 4214 4964 +750 Misses 114 114 ``` | [Files](https://app.codecov.io/gh/pysal/momepy/pull/570?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal) | Coverage Δ | | |---|---|---| | [momepy/\_\_init\_\_.py](https://app.codecov.io/gh/pysal/momepy/pull/570?src=pr&el=tree&filepath=momepy%2F__init__.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal#diff-bW9tZXB5L19faW5pdF9fLnB5) | `100.0% <100.0%> (ø)` | | | [momepy/dimension.py](https://app.codecov.io/gh/pysal/momepy/pull/570?src=pr&el=tree&filepath=momepy%2Fdimension.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal#diff-bW9tZXB5L2RpbWVuc2lvbi5weQ==) | `98.7% <100.0%> (ø)` | | | [momepy/diversity.py](https://app.codecov.io/gh/pysal/momepy/pull/570?src=pr&el=tree&filepath=momepy%2Fdiversity.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal#diff-bW9tZXB5L2RpdmVyc2l0eS5weQ==) | `98.4% <100.0%> (ø)` | | | [momepy/functional/\_diversity.py](https://app.codecov.io/gh/pysal/momepy/pull/570?src=pr&el=tree&filepath=momepy%2Ffunctional%2F_diversity.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal#diff-bW9tZXB5L2Z1bmN0aW9uYWwvX2RpdmVyc2l0eS5weQ==) | `100.0% <100.0%> (ø)` | | | [momepy/functional/tests/\_testing.py](https://app.codecov.io/gh/pysal/momepy/pull/570?src=pr&el=tree&filepath=momepy%2Ffunctional%2Ftests%2F_testing.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal#diff-bW9tZXB5L2Z1bmN0aW9uYWwvdGVzdHMvX3Rlc3RpbmcucHk=) | `100.0% <100.0%> (ø)` | | | [momepy/functional/tests/test\_distribution.py](https://app.codecov.io/gh/pysal/momepy/pull/570?src=pr&el=tree&filepath=momepy%2Ffunctional%2Ftests%2Ftest_distribution.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal#diff-bW9tZXB5L2Z1bmN0aW9uYWwvdGVzdHMvdGVzdF9kaXN0cmlidXRpb24ucHk=) | `100.0% <100.0%> (ø)` | | | [momepy/functional/tests/test\_diversity.py](https://app.codecov.io/gh/pysal/momepy/pull/570?src=pr&el=tree&filepath=momepy%2Ffunctional%2Ftests%2Ftest_diversity.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal#diff-bW9tZXB5L2Z1bmN0aW9uYWwvdGVzdHMvdGVzdF9kaXZlcnNpdHkucHk=) | `100.0% <100.0%> (ø)` | | | [momepy/functional/tests/test\_shape.py](https://app.codecov.io/gh/pysal/momepy/pull/570?src=pr&el=tree&filepath=momepy%2Ffunctional%2Ftests%2Ftest_shape.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal#diff-bW9tZXB5L2Z1bmN0aW9uYWwvdGVzdHMvdGVzdF9zaGFwZS5weQ==) | `100.0% <100.0%> (ø)` | | | [momepy/tests/test\_utils.py](https://app.codecov.io/gh/pysal/momepy/pull/570?src=pr&el=tree&filepath=momepy%2Ftests%2Ftest_utils.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal#diff-bW9tZXB5L3Rlc3RzL3Rlc3RfdXRpbHMucHk=) | `99.2% <100.0%> (ø)` | | | [momepy/utils.py](https://app.codecov.io/gh/pysal/momepy/pull/570?src=pr&el=tree&filepath=momepy%2Futils.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pysal#diff-bW9tZXB5L3V0aWxzLnB5) | `98.8% <100.0%> (-<0.1%)` | :arrow_down: |
martinfleis commented 7 months ago

Some timings:

no mode, no quantiles
# new: 141 ms ± 626 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)
# old: Wall time: 12.7 s

with mode, no quantiles
# new Wall time: 15.1 s
# old Wall time: 30.2 s

no mode, quantiles
# new Wall time: 8.54 s
# old Wall time: 14.4 s

with mode and quantiles
# new Wall time: 23.6 s
# old Wall time: 30.8 s

So we're faster in all cases. How much depends on the use case.

martinfleis commented 7 months ago

Merging but have numba version of the same in works. Will do follow-up.