-
Notifications
You must be signed in to change notification settings - Fork 56
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
ENH: describe as a replacement of AverageCharacter #570
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the potential swapping of values
and graph
I think this looks pretty nice and lean.
Co-authored-by: James Gaboardi <jgaboardi@gmail.com>
Some timings:
So we're faster in all cases. How much depends on the use case. |
Merging but have numba version of the same in works. Will do follow-up. |
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 intomomepy.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 originalAverageCharacter
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.