[BUG] set viz defaults for LineStrings in lisa_cluster
#140
Conversation
slumnitz
commented
Aug 5, 2021
slumnitz
commented
- add check for LineString on zeroeth index
- change visualization defaults for gdf's with LineStrings
- add tests
* different plotting defaults dependent on geom_type
splot/tests/test_viz_esda_mpl.py
Outdated
| def test_lisa_cluster(): | ||
| df = _test_data_columbus() | ||
| moran_loc = _test_calc_moran_loc(df) | ||
|
|
||
| fig, _ = lisa_cluster(moran_loc, df) | ||
| plt.close(fig) | ||
|
|
||
| # test LineStrings | ||
| df_line = _test_LineString() | ||
| moran_loc = _test_calc_moran_loc(df_line) | ||
|
|
||
| fig, _ = lisa_cluster(moran_loc, df) | ||
| plt.close(fig) |
There was a problem hiding this comment.
Do you want to check the actual figure?
There was a problem hiding this comment.
I am definitely open to suggestions how the figure could be checked! :)
There was a problem hiding this comment.
One option is to get the attributes from the plotted collection as we do in geopandas https://github.com/geopandas/geopandas/blob/64598b6f0d872baf09657c27a9aa4c87d86d9131/geopandas/tests/test_plotting.py#L169
The other is to keep a reference image in tests and check against that as matplotlib does in some cases https://github.com/matplotlib/matplotlib/blob/ec126b0175c657f0adb5ca635eeabad7c142bb0b/lib/matplotlib/tests/test_colorbar.py#L188-L190
slumnitz
force-pushed
the
line_poly_lisa
branch
from
8b2a11d
to
e427b9d
Compare
Co-authored-by: Martin Fleischmann <[email protected]>
slumnitz
force-pushed
the
line_poly_lisa
branch
from
ac21846
to
7dcb600
Compare
|
@slumnitz @martinfleis Is this failing because the example data wasn't downloaded properly? Can we circle back and try to get this worked & merged? |
No, the test was wrong. I've pushed a fix. |
Codecov Report
@@ Coverage Diff @@
## main #140 +/- ##
==========================================
+ Coverage 85.94% 86.05% +0.10%
==========================================
Files 18 18
Lines 1217 1226 +9
==========================================
+ Hits 1046 1055 +9
Misses 171 171
Continue to review full report at Codecov.
|
|
@slumnitz This seems to be in good shape now (thanks to @martinfleis!). Would it be possible to get it merged, and a release cut & published to PyPI soon? |
would be nice to get #143 in if we want a release. |
|
@martinfleis and @jGaboardi since I opened the PR, please take on the merge, so we practice non self-merging ;) |
