Repository navigation
Expose measure classes at the top-level namespace (GL#38) - #117
Conversation
Previously only wolfson's lorenz_curve/wolfson accepted pandas objects (via the consistent_input decorator); Gini, Atkinson and Theil supported pandas only by coincidence and could break on Series (e.g. Series has no .ravel()). - utils: add _resolve_array(data, column=None), the shared normalizer for list/tuple/ndarray/Series/DataFrame input. DataFrame requires a column selector (str -> 1-D, list[str] -> 2-D for Theil's (n,t) case). consistent_input now delegates to it. Both carry doctests. - gini: Gini and Gini_Spatial gain a column= kwarg and normalize x. - atkinson: atkinson() and Atkinson gain a column= kwarg and normalize y. - theil: Theil, TheilD and TheilDSim gain a column= kwarg; y and partition are normalized (partition may now be a Series). - tests: test_utils.py exercises _resolve_array plus list/Series/DataFrame equivalence for Gini, Atkinson, Theil(D/DSim) and wolfson. schutz/pen/pengram/polarization are pandas-native by design and unchanged.
…res (GL#104)' (#2) from support-pandas-104 into main Reviewed-on: http://polaris.tail065cc.ts.net:3000/serge/dev-pysal-inequality/pulls/2
Investigate GL#38: promote the measure classes so momepy and others can do 'from inequality import Gini, Theil' instead of reaching into the submodules. - __init__: re-export Gini, Gini_Spatial, Atkinson, Schutz, Theil, TheilD, TheilDSim and S; also import the previously-missing pen and polarization submodules so all measure modules are uniformly reachable from 'import inequality'. Add __all__. - Purely additive: submodule paths (inequality.gini.Gini, ...) are unchanged, so no FutureWarning is needed (contrast the 2022 issue thread, which predates pandas/joblib/tqdm becoming core deps). - Only classes are promoted, not the module-name-colliding functions (atkinson(), pen()); inequality.atkinson / inequality.pen stay bound to the submodules. wolfson()/lorenz_curve()/pengram() likewise remain module-qualified. - tests: test_namespace.py checks identities, submodule access and __all__. Open for review: whether to also promote the functions (bigger change, needs the module/function name clash resolved) and update docs/api.rst to the short names.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #117 +/- ##
=======================================
+ Coverage 94.6% 95.5% +0.9%
=======================================
Files 9 9
Lines 405 419 +14
=======================================
+ Hits 383 400 +17
+ Misses 22 19 -3
🚀 New features to boost your workflow:
|
jGaboardi
left a comment
jGaboardi
left a comment
There was a problem hiding this comment.
LGTM, let's also either Eli & Martin's eyes before merge thought
martinfleis
left a comment
martinfleis
left a comment
There was a problem hiding this comment.
The API exposure is fine but the commit history here is garbled. I think you wanted only ce60dde in this PR.
Promote the functions too? Needs the module/function name clash resolved (rename the pen module, or accept inequality.pen becoming the function).
I think we're stuck with current API here.
Update docs/api.rst to short names (Gini vs gini.Gini)?
If that is the preferred API, then yes.
Is S too terse for a top-level inequality.S
It is not ideal but it is consistent...
Btw, what does GL in the title stands for? I understand that it is a reference to the issue but what the letter mean? I could not decipher it.
Also, I would like to point out that reading these generated PR descriptions is a massive pain and I'd rather avoid that if possible.
GitLab style issue.
Got it. Will correct in future. |
3 participants
Investigates and addresses #38 — promote the measure classes so downstreams (e.g. momepy) can
from inequality import Gini, Theilinstead of reaching into submodules.Changes
__init__: re-exportGini,Gini_Spatial,Atkinson,Schutz,Theil,TheilD,TheilDSim,S. Also import the previously-missingpenandpolarizationsubmodules so all seven measure modules are uniformly reachable fromimport inequality(before this,inequality.polarization.Sfailed after a bareimport inequality). Add__all__.test_namespace.pychecks identities, submodule access, and__all__.Notes from the investigation
inequality.gini.Gini, …) are unchanged, so noFutureWarningis needed — contrast the 2022 issue thread, which predatespandas/joblib/tqdmbecoming core deps and_indices.pybeing removed (remove deprecated_indices.py#101). The original "distinguish from the functional_indices" rationale is now moot; the ergonomic case remains.inequality.atkinson/inequality.penare submodules that also contain functions namedatkinson/pen; re-exporting those functions would shadow the submodules and breakinequality.pen.pengram+docs/api.rst. Sowolfson(),lorenz_curve(),atkinson(),pen(),pengram()stay module-qualified.seaborn/mapclassify);pen.pyalready lazy-guards those via_check_deps().Open questions for reviewers
penmodule, or acceptinequality.penbecoming the function).docs/api.rstto short names (Ginivsgini.Gini)? Left untouched here to avoid churning generated files.Stoo terse for a top-levelinequality.S?Full test suite + doctests pass;
ruff check/ruff formatclean. Draft pending the discussion above.