From 905fa4642f7346a2c726558e0dc34b6b58c5aa64 Mon Sep 17 00:00:00 2001 From: Tim Treis Date: Mon, 17 Nov 2025 14:06:48 +0100 Subject: [PATCH 1/2] polygon fix --- src/spatialdata_plot/pl/render.py | 3 +++ src/spatialdata_plot/pl/utils.py | 32 +++++++++++++++++++++++++++++++ tests/pl/test_render_shapes.py | 14 ++++++++++++++ tests/pl/test_utils.py | 19 +++++++++++++++++- 4 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/spatialdata_plot/pl/render.py b/src/spatialdata_plot/pl/render.py index 76e57d82..498d8efd 100644 --- a/src/spatialdata_plot/pl/render.py +++ b/src/spatialdata_plot/pl/render.py @@ -57,6 +57,7 @@ _prepare_transformation, _rasterize_if_necessary, _set_color_source_vec, + _validate_polygons, ) _Normalize = Normalize | abc.Sequence[Normalize] @@ -184,6 +185,8 @@ def _render_shapes( ) shapes = _convert_shapes(shapes, render_params.shape, max_extent) + shapes = _validate_polygons(shapes) + # Determine which method to use for rendering method = render_params.method diff --git a/src/spatialdata_plot/pl/utils.py b/src/spatialdata_plot/pl/utils.py index cccb587e..47197633 100644 --- a/src/spatialdata_plot/pl/utils.py +++ b/src/spatialdata_plot/pl/utils.py @@ -1246,6 +1246,38 @@ def _get_linear_colormap(colors: list[str], background: str) -> list[LinearSegme return [LinearSegmentedColormap.from_list(c, [background, c], N=256) for c in colors] +def _validate_polygons(shapes: GeoDataFrame) -> GeoDataFrame: + """ + Convert Polygons with holes to MultiPolygons to keep interior rings during rendering. + + Parameters + ---------- + shapes + GeoDataFrame containing a ``geometry`` column. + + Returns + ------- + GeoDataFrame + ``shapes`` with holed Polygons converted to MultiPolygons. + """ + if "geometry" not in shapes: + return shapes + + converted_count = 0 + for idx, geom in shapes["geometry"].items(): + if isinstance(geom, shapely.Polygon) and len(geom.interiors) > 0: + shapes.at[idx, "geometry"] = shapely.MultiPolygon([geom]) + converted_count += 1 + + if converted_count > 0: + logger.info( + "Converted %d Polygon(s) with holes to MultiPolygon(s) for correct rendering.", + converted_count, + ) + + return shapes + + def _collect_polygon_rings( geom: shapely.Polygon | shapely.MultiPolygon, ) -> list[tuple[np.ndarray, list[np.ndarray]]]: diff --git a/tests/pl/test_render_shapes.py b/tests/pl/test_render_shapes.py index 8fc39582..6728f64f 100644 --- a/tests/pl/test_render_shapes.py +++ b/tests/pl/test_render_shapes.py @@ -115,6 +115,20 @@ def test_plot_can_render_multipolygons_with_multiple_holes(self): fig.tight_layout() + def test_plot_can_render_multipolygons_that_say_they_are_polygons(self): + exterior = [(0, 0), (0, 1), (1, 1), (1, 0), (0, 0)] + interior = [(0.1, 0.1), (0.1, 0.9), (0.9, 0.9), (0.9, 0.1), (0.1, 0.1)] + polygon = Polygon(exterior, [interior]) + geo_df = gpd.GeoDataFrame(geometry=[polygon]) + sdata = SpatialData(shapes={"test": ShapesModel.parse(geo_df)}) + + fig, ax = plt.subplots() + sdata.pl.render_shapes(element="test").pl.show(ax=ax) + ax.set_xlim(-1, 2) + ax.set_ylim(-1, 2) + + fig.tight_layout() + def test_plot_can_color_multipolygons_with_multiple_holes(self): square = [(0.0, 0.0), (5.0, 0.0), (5.0, 5.0), (0.0, 5.0), (0.0, 0.0)] first_hole = [(1.0, 1.0), (2.0, 1.0), (2.0, 2.0), (1.0, 2.0), (1.0, 1.0)] diff --git a/tests/pl/test_utils.py b/tests/pl/test_utils.py index a9296d2e..b237666a 100644 --- a/tests/pl/test_utils.py +++ b/tests/pl/test_utils.py @@ -1,13 +1,15 @@ +import geopandas as gpd import matplotlib import matplotlib.pyplot as plt import numpy as np import pandas as pd import pytest import scanpy as sc +from shapely.geometry import Polygon from spatialdata import SpatialData import spatialdata_plot -from spatialdata_plot.pl.utils import _get_subplots +from spatialdata_plot.pl.utils import _get_subplots, _validate_polygons from tests.conftest import DPI, PlotTester, PlotTesterMeta sc.pl.set_rcParams_defaults() @@ -136,3 +138,18 @@ def test_utils_get_subplots_produces_correct_axs_layout(input_output): assert len_axs == len(axs.flatten()) assert axs_visible == [ax.axison for ax in axs.flatten()] + + +def test_validate_polygons_converts_holed_polygons(caplog): + shell = [(0.0, 0.0), (0.0, 1.0), (1.0, 1.0), (1.0, 0.0), (0.0, 0.0)] + hole = [(0.2, 0.2), (0.2, 0.8), (0.8, 0.8), (0.8, 0.2), (0.2, 0.2)] + holed_polygon = Polygon(shell, [hole]) + plain_polygon = Polygon([(2.0, 0.0), (2.0, 1.0), (3.0, 1.0), (3.0, 0.0), (2.0, 0.0)]) + shapes = gpd.GeoDataFrame({"geometry": [holed_polygon, plain_polygon]}) + + with caplog.at_level("INFO"): + validated = _validate_polygons(shapes.copy()) + + assert validated.iloc[0].geometry.geom_type == "MultiPolygon" + assert validated.iloc[1].geometry.geom_type == "Polygon" + assert "Converted 1 Polygon" in caplog.text From 37958af81d313aec19b359c19f15dd9d9e83ec7a Mon Sep 17 00:00:00 2001 From: Tim Treis Date: Mon, 17 Nov 2025 14:14:21 +0100 Subject: [PATCH 2/2] added image from runner --- src/spatialdata_plot/pl/utils.py | 2 +- ...ltipolygons_that_say_they_are_polygons.png | Bin 0 -> 7503 bytes tests/pl/test_utils.py | 19 +----------------- 3 files changed, 2 insertions(+), 19 deletions(-) create mode 100755 tests/_images/Shapes_can_render_multipolygons_that_say_they_are_polygons.png diff --git a/src/spatialdata_plot/pl/utils.py b/src/spatialdata_plot/pl/utils.py index 47197633..1ba3e3ba 100644 --- a/src/spatialdata_plot/pl/utils.py +++ b/src/spatialdata_plot/pl/utils.py @@ -1253,7 +1253,7 @@ def _validate_polygons(shapes: GeoDataFrame) -> GeoDataFrame: Parameters ---------- shapes - GeoDataFrame containing a ``geometry`` column. + GeoDataFrame containing a `geometry` column. Returns ------- diff --git a/tests/_images/Shapes_can_render_multipolygons_that_say_they_are_polygons.png b/tests/_images/Shapes_can_render_multipolygons_that_say_they_are_polygons.png new file mode 100755 index 0000000000000000000000000000000000000000..f5475dd4b4d8a2fa84fd545278919e7169af3ffe GIT binary patch literal 7503 zcmc&(cTiLNy4|2yuwVmJq#OaIV*!y4N(&%J??pLC2Z4lM6-B8^J<^*}L+AmiqBN0i z2vP;4OOqZ1-k0OKckaA%-~IF53>jwj?ELnxtZ#j5t^NG2iu^I^^VA4}978F{s3Qo) z3;6OJIsjMZ@e}p%PXsNii`H;>fOfs-WRBdqhkj`5fVQ=|&*Eb4bDjia5+(KV4y54MTGG;Qdi7twc0PF@-#DD{YAzhD(PL7~ z^Hx%sXcr`1$9a@{-)bd4w64m@tvo**0)9IRnzm@xV2RXSWTLXYSeyUUI@h3hCQhb&YsN(c%HQdQO5 zdw*cav57G#Je-Ij8fAJeR_pj~4~*P$e^%#r$_j(9oZ0<3B{|n5xm)<-$B)&{+cY!H zFIn}=AEs6rJA^w+Y=6H#) zsF-3BPE^_JJ0>)k;<->EfAIK)Lwy@bo(o-BYUw`*G%{7R6cvMpJr~9I`e0)-VbM0L z(_KL!A)1<+QDGN^(^%!h3>&&A_R&*O4y?F|iNIv?Yj;V0%WCOzR?RPxB**6G=V9}d zJ2(HhYW8_}nCZ$5kJagvodlK4FlI@i(c|3 z-fPh<_bB!;*{~E;&*5r~U%q_#!%d80+{A17Nqe&7$hcVG`_$gv-jwCjmsOfaAMOuH z6m!33S=-yI?Y&Vs63sS0RH?HE_*%=9+~NGH_&PI!vYtW*MoYaNXx(38m6MZ0eHNFM zRsO9?bg@cfdt;G?BM`=##^Q&&6J3YT6)f)VY_Cnknvn34n_u@aihF3f4mt+1W#{H9 zojiGRmkNXOn8V^%XEKxHnCPy2Rbx4Hq(fpmZM1#J7S4EgTu<&15y2N$Sc#pQ}TT5gqW=?x&C3#QN#ii_qU4Mq;&rzDVEB9_C zGmDx=A4(2ys$dl(^!9RHzfRsu;*jfz9p(bd_r6)`->2FWKen|ILWPF2`ucQVXJ*bt z>lmxKx^9?H#Y9ClPdIVH4)Db)!F=ZeKTPvysw8%ghfC(?@{JkZ|=mu zf!b`Hb(7L9BADDG?`um-i^tYP>;zh5tzC3+`jthJnzwf~kcVf2j-Fn2Rh5|d*0;#G z>lR_w*48DqU#=ZvmN5MC<;#xS{%LNgUu%1NZE0!${{H@`7cWwTohPStYDv)-FJ8aIW|7NuaV!YryPsLs$Qp*DlZ~jB<+#Lg82n}cfIcXaYdTHX7hftudkZg z%b=j3p94+d%rtH?RDMf@uCCN#iw=E8v$|q){{#tdedozfT|^UK?9TSaT)z!&-f`rJ z&Ur>gMnz@i)o_#YYbXVU@@X;r056czh>J)fy|?|TPoIfIn~zbP+D(TT5;{6bBfdH* z7G;%BFzes-Ccb^ks;JQO+*lyMa``Lvo|J|=;4rHtHz<+vYh_#~aKzk$%wBgFYxip4 zfY7bHctaNzpdIKZrQX}?e#=Eg*8nfvoSfOXF-j@jVgheo6O*U1mX?-&r3)62J?y<< zv}Xh0VU-K%{4vq0WoVe^Jk`EsVo>}bl*#QY1=hG)A6BVtXNPr}@26d=K7mLrl2O@_ zBpy-Y{$qBdW~16KnMPt$s#{2hb7XhVlwZF@p`oDx>aaUcueij9$Y(e7R;!|-V#ntw za)Qrx?GBo|@EMan59ul0)b)~6r%w~QyYFK{C_1!tSP(z|$CMPv*;FY6k^7g<_y2wA zgEm$7=g%4S^|$UMif94W6;)Jf(%iSUwt}8LJ4Di}_3<7#aKP^hofXD!_@>upK~8Qi zW_uprI<=BrUA+@Db%~vwTtJpb5z|I(s%s9zVsM-a5!6SHlv;EoaVIe_GGbp{w^-K_ zK+d9VF-L5gPZ%^%`4!x-+zlNZ9OMCh&dTDrlO(2xt6q;IyhQ4H;jY%tcKD$;Gd-Ey z;^Ib$iHThN{KOzy?r+O(tDiMg^%!XoTdTWgnsc;rDa)qX;xD80@%D+@SpC<1eSImy zva+&VTwFgKTrrsJnwlC_=i`@#hK3S4PK*1*NfOBj70C}`Fgl0qD^ z#}C@$eFtXH)&SC-b64{SM543A?Pn{8xsS#PzDSahkwk83R{hSMP_`N9u4+0uu`UaPoT#{~W=|s`B1*iroM`Fk=LRdBBQngI!e+>% z|CmN1>+tDI)1O{l2YTpX-rJO~UhP&2dj7odyBkv1ZmY#|SvBc%nrtv-BcH9qaFrgM z$PbTd_}e80(lGPEw$U5ANt18tOudbP4wI9SA)^$_H&6VDoKU&wC6U}Qd&1=_T7=Nw zZ;c5lc$em2=_qlV`|4FSUS3}9rn5^2=FcBOzRuodK@iq6hmg8MRFue-TmA?V^8bIS zFJCGyE>2TdH!E8s6V6T%C|gGi4@-iY1=;6RVc1`0XD~uV<#)xRl2Z5=5Tg?k@>(J< zn=Ds-{HQoFF=4eZSdoLpYH4f7#PAxt0gkGs>7kU`3|ayWm3w=&acgBt@b_k+Y8`lx zxaT^s&Q{TiAzv&IiI$$;40CIHdv8~|{92bhvyn?p&y1#xO|GN;{u~^R7{#fP7k2W- z&E(YRKmN#yiJ{9iJL*Wvs zrYs;Jpb*jB-A&nO#23q!U07H+a-G6&NLHUQMY05sSBm(Po*9pWYyf_)WtAQo4BKyN z0#H>xLra^TmDOB3L?FB-kvxu^eiFN|V7~+M7%w8HJG%0G_XY>`xB6%~dHFz~n4NL@ zeC1Q8PCc3|AF{6)vfnG~nfd+YwE#dUSySYi1%$3lb*ya3)H0m0VWIDpsHyJl>gfj{ z{5T-%pOKNlAt_soS`PAUf?X zR8Bv>p?LL6I849SuTQqcM<*xqJb?lUb?Q^!LqBJWAW2E5ANmxVsCZ0;0vzY{Ls|I&Bxb1dv@O~3yVw<5s|zw ziv6ilS6CW8eAq88Y+3;2bLP+iHv4-N{|%7;SypA-FW$VlWd7g*7ayNWx?E`Y$7pU0 z7?{@f){YJxpqb7N(%RhiZ-PibvO?~@)kY|YsFyEO3(cCWo5C2aZEPY3=CuKrP$hh# zlAN3<0KT*t;x}BZ^wY;&&=Z%q*4;o$apgrLW zvl<1<_b;<_+CW;@u3h^%@f!*R3fWCzFr`mg5YweXEq?$l2T+A6&$Vmnk(chMo0*N9 zPYDSLwT?dIfC`PgtlDdXtM1kL$5@`8l>p<%jR0d%N_@f_Wdrs-t1~@7l=`W5JRV0AlfVP>STWYOUw_fGG!iJjb#r6QDpJ8BNSn~LX5FvvOhg)1p zc-#W!!qaQMxI?0+c6SZuqkR2gqNJ~}F>a%1dwEO77dGJLh2t1BdVeS$kGC27@Vt80 zJD8qd8)WivW{JFW*B|V!nslk0aH-rGP|4KLc%wYC=J|E(S8Wt9nbeLxWk3&Mh-+qP z{&TJTwV#*AoA@g_fHk(f(nh{mA7g|4u*ANfTbl)gC~d=94GQ0}vvcmgeQs5iVTbre!u|XA z^KM>13Iq+X{($_sHldNvWIg=k2t5}l>?fMF&X=!Oa zHb(>bMe#*N>hA9DaiXsIP=e9X(XY*0V`kQfCX0dJkP|+(r3Ze@=Hfn%FHe7c_xQZH zM+qC}rAu;0=y>jesRmEdJ3eTUP_=BTr(@O&5>T1Bq zYKwMtC3lg(*Bm?;^~($#{o^v~@ZrPK&LFM+MP0q=TIO9_Tl-6nmFe$8DJdP%$1QnO zi#UB#=p;4br%>-~7{#%n2*TH)O%?I#RW_*nmht7yCYQxwEC2v1YV9UJJG*Rlc6NV_ zw`ZLjXtM5}p6IBk&layJSPZ}9NKsQ$ligZE!Ugb~IVC0J{N-ayOBXQG&Ni<}jp56e z{;pGCq|<|fsLAr5>_dOG^6U9 z1C)@Pk-=sv`KxbTDYNXl2qG^xCr7dPL0fmeQDq4j;fcw~^{L+wslI2w1W2F%FG)e( zf3J2q3!>GW!b(a)L>BbaXr~XdR`UZZIFk{YYzh`J5HVjB6GbqJ=gyrwFY1!#^1Uy$tn3D8kJ7TToV2vDqLs*q2u)*S zCV>OU6|;Cs_cemf#IhTD>8o@olem7wnbW7!sgBTP<>#va(N%u?3F%f_f^e~98ksEP z4Rj&J``FyP6dTB9ZealpUIo%Nx=Z`~-nHELx5!T%`QN@m-$a(mPx{>rd_n)f04*~! zv%tc`o1c4oU0N#eLoV@eRFm^pSJ4<3mmJ_$Ztc7mP2*h}s^SFs=M*eedfF@oWwu@E z;SwHScF3y(f;A?1V)aY8Og)FI+#@nDbG=rC94#REV{saqni*+n7x!F%oMz|da9js> zb1N(3SI*r^P}O}`MA_2TR-Gn0?>OFwB26l$r>C<{udT1U+)21uNL;LSTfQG16EnLs z`X1baT<4rU-oPF|P&o z8R+GW!f;9hCT3lY9GKf({RaF{uW#p`Y ztp*Df6_uib!d(@WFyJ{Yef=D;hT!NdNvkuvJ7kg9+`MP3%P8ui>^kC&Cr3O&!)~|9 zPP@vj@MCN1+Msg>P+w!vdkSQrxXt8V!+&T^(Zsnm2O0=ljtAc6uUdMfk|dT5)Hd5| z;Gl&K_TDK<=@UL*=~V_YOkW27yct5&rI~;dBvX-;w{Ki}|N-dWQY8`2gd@ ziC~}x(xMU)26MOA+1bI!1cimgv`%!F*$t2`aUe%2%eY>=52VR``&J)JPn?*$6K+yl zTN@m&DkKzI5J(gi6_G=!>Rr;DcN85`Z}6*Ck13MX&+VsPJ?`+J{$h*pxcGP+ne+c$ z>eSlqDw1ezXD9eE)D+k^?wWZJnTGxrOwfN!$HO>;zP>DWrI`I8{6=pz1CKLygBH1U zk_I_IYe4%f`{~mID0O{({kn2!Fl^A7);%vF1(nijERf|!WNdD3`Z)%^{|yBYnp;#r zOg&#}*KC0$Tb)i1H+HE7^pKf!=f!j_WK9qhe;ZkM6NC8ZJMrjf9_S`uoxg%2$kGA` zCvD$#lM9CccA39&1hUNhS#Lf=D>O*GsJbWIkpTe#DZ+o%N?V#f+2{tZoYVi=G!|W0 zA_Wza{gPFFeQVoLciYm|_UPuyNK&sHg2W1O^h&9#t4GDf z??bkWR;e@aZ{NPv)X-o>k-kN$eg+c;4o5NK!KXh)`5#TR#^#on>q2n3(qZ&}1xYWI zwnhf@$~_?U8uNlAXys$yY;XW%eTpbl#k5Vlkb^dGDjTPyqzR-+r1eyCiqs*wR~5!2 zo)Z;y=1+yt+B!Gahn$FkwhYOU*)P)*PC4^B*ZbmX$S?HC4V@KG6^|Q1VO>Izey((Ei-y@P z9hRxJw6r`#YV`g6_l!$-;y(1exXMz!vp(>4GR_9Q(k5iUXkM2l)yWUt;BnGZ9`BWe zYe`Q-{NJReN;BNB?S!OB;Qoh0x#LH5AJ<61JD;>?&z`mJ8g4d<`Yo+D(3@E9{@j98 z`;oENvMk(X`FIysK$?4A_c-zW=v{ev`A#=5Lzh$%`u6<&f^T}0O2}`haJ9RCYK&5% zu?;4J!mg8E!B_mGTY(Z;{>=T7KhI!&yoyvmiC}f2BO}v6_wB&jrY^`uv0(yVkC6;? zr8S5y(HippVo91nf6~pnMTDmUdGS zuS9TaBq*}+ib0qzVfg@0QsWWiN