From 516e799bb9753ed45b9e122b8ebb5fd446320866 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Wed, 24 Jun 2020 00:40:12 -0400 Subject: [PATCH] Fix a bug when passing data to GMT in Session.open_virtual_file() (#490) External programs like PyGMT can pass dataset/momory to GMT. By default, GMT can read, modify and free the momery, which sometimes can cause crashes. Issue #406 reports an example in which PyGMT crashes. The issue was reported to the upstream (see https://github.com/GenericMappingTools/gmt/issues/3515 and https://github.com/GenericMappingTools/gmt/pull/3528). It turns out to be a API user error (i.e., a PyGMT bug). As per the explanation of Paul, external programs like PyGMT should always use `GMT_IN|GMT_IS_REFERENCE` to tell GMT that the data is read-only, so that GMT won't try to change and free the memory. This PR makes the change from `GMT_IN` to `GMT_IN|GMT_IS_REFERENCE` in the `Session.open_virtual_file()` function, updates a few docstrings, and also adds the script in #406 as a test. --- pygmt/clib/session.py | 13 ++++++++++--- .../baseline/test_plot_lines_with_arrows.png | Bin 0 -> 21125 bytes pygmt/tests/test_clib.py | 9 +++++++-- pygmt/tests/test_plot.py | 17 +++++++++++++++++ 4 files changed, 34 insertions(+), 5 deletions(-) create mode 100644 pygmt/tests/baseline/test_plot_lines_with_arrows.png diff --git a/pygmt/clib/session.py b/pygmt/clib/session.py index 39999268d2c..18f0c78887b 100644 --- a/pygmt/clib/session.py +++ b/pygmt/clib/session.py @@ -922,6 +922,9 @@ def open_virtual_file(self, family, geometry, direction, data): direction : str Either ``'GMT_IN'`` or ``'GMT_OUT'`` to indicate if passing data to GMT or getting it out of GMT, respectively. + By default, GMT can modify the data you pass in. Add modifier + ``'GMT_IS_REFERENCE'`` to tell GMT the data are read-only, or + ``'GMT_IS_DUPLICATE'' to tell GMT to duplicate the data. data : int The ctypes void pointer to your GMT data structure. @@ -950,7 +953,7 @@ def open_virtual_file(self, family, geometry, direction, data): ... lib.put_vector(dataset, column=0, vector=x) ... lib.put_vector(dataset, column=1, vector=y) ... # Add the dataset to a virtual file - ... vfargs = (family, geometry, 'GMT_IN', dataset) + ... vfargs = (family, geometry, 'GMT_IN|GMT_IS_REFERENCE', dataset) ... with lib.open_virtual_file(*vfargs) as vfile: ... # Send the output to a temp file so that we can read it ... with GMTTempFile() as ofile: @@ -1086,7 +1089,9 @@ def virtualfile_from_vectors(self, *vectors): for col, array in enumerate(arrays): self.put_vector(dataset, column=col, vector=array) - with self.open_virtual_file(family, geometry, "GMT_IN", dataset) as vfile: + with self.open_virtual_file( + family, geometry, "GMT_IN|GMT_IS_REFERENCE", dataset + ) as vfile: yield vfile @contextmanager @@ -1167,7 +1172,9 @@ def virtualfile_from_matrix(self, matrix): self.put_matrix(dataset, matrix) - with self.open_virtual_file(family, geometry, "GMT_IN", dataset) as vfile: + with self.open_virtual_file( + family, geometry, "GMT_IN|GMT_IS_REFERENCE", dataset + ) as vfile: yield vfile @contextmanager diff --git a/pygmt/tests/baseline/test_plot_lines_with_arrows.png b/pygmt/tests/baseline/test_plot_lines_with_arrows.png new file mode 100644 index 0000000000000000000000000000000000000000..eddfd08559bd9c604406681324be845c04bfae0e GIT binary patch literal 21125 zcmeHP30PCfmcF>f6|EQ zq!j}Q24qV@BhY9B5e-X(ghfyj2oNDaAep*1((`?9zWMsiyk6&VzfUhYwce_~PMve= z)JZ&HWwvhh*3}RMtvhz~hz$gxh~Qu1$|Znecy5_4c&!RL>JSP+`xM}R5}17B2tbq# zGqDe|4fGC+^awc%nVmSk-Rzsd(6CU?5TBs1?YjH+?{}NOUj#whp<_qB`Wi#v^t1)% z{j}u5$+F1Opq1Oc<-OaLb0uL{N}{*ti`Wy_Et0&}ZP)fqXVhbSZ;l4( z#DJC{=->~ODLJa2AMv%LAF(HKU=NX0Yla%DA*T?qQp;Fh?NYkZvRPs%=;g|>jSds1 zvu9X*AL6=twd8^8{4|?caxuy1+}BWXzu-(`@!z()5+|P?%hNE$?;&U|A*3pD^WvEU zvkkN($_)u>P=Qs-?YMOkbwkd#W!BZRzFhA9<1q!QuERtC-K4V163b|6YSKwA@`a$@ z6HLBF>ox&7GI0q+4KE$spf~N`!Y{WmSAHARKmGJQmJ$1-CU|5J z)Td%a)HVqNeWfeV#?rwsY2Bd_hl0_!rHi|EQuYuKFg*R971$?brO)frG`?x z{A))g+aD{oZi1j1b=jDr=H)aRYKx^X9s3ys^;@RT2?qp9O6f!KOQ9h1bmxZRTGZJ7 zUA$XfPLRFHP28(>Yk`C8$Ny5akzs`n6tC$8#;Ai_08 zyFT5_ZN9{w*|eQ7H1LXE&7peJ7k2Sg>}5AVEaKx{2y&M$9n_VL*+?Wc_pHwUoUaN& zPs;AEf*}2CinueeW0NHMuH1)dHYz4ka2@B&(rXT(+-&{)H(zMpxgEO=CYH!LOr(U3 zy?*7`8W>Nikbr57tBS)f>g$& zy_yx549Ck$CXA402FXivD_NBY1SYKke3xB@Jkxghf+4BPg!87vx3`_ePVi#ON z$6(sToU0uGG(n}3AQ>0m?j*(Whal&$s*+|xVsEOz$&j@Y!mk1-f$>nVZwO3CP}|^< z@f17d)O&_mmsdgf8;TZ1hfulC3}hi_D1)%u%1WV_(sgPb1f%@RCAq0HP=1;qLWi#c za20R!luyVGmT~fvZk228hzdKk6rz4nI@nWH-&exl?0UE({23*YxNdBmsc?U(!uR*- z(enyI=u;=lkOe36hfV?}PhgW#Bj7OHT)W zL(O5JHt<|~&qCfi+rdf$&%a*56a^=oL37m&iX8!OWLHr{LM-|VfHZX7m(T|BtJG4L4$06>kS=Y{{Ks#Qa*?kg z0lYOe-#j2M4}hDJl0xPZ-H=bGhDU}Cp~A-oezHoK9GULdNzVWLfcWE+Mn`EW?^q;y zl0w$zrg5F8^zNmm%i+(a7TUke)OM4P8Df#I418lQ9wg?bh#`Od=^2)}a(-Gp;P7@A zz@UTN49DKg3K!|OF+szAIWlgUa~1*S0yqV_15&gJQbw8Wv9WzzdiNcA)&`-0FVUqV z+@*qla0+c3qaT10<#sD(lvu=srcHa>%FIyWS5-gJHcM0h|7U#%|Ba%-e^);d*yn2m zcGg)}3kU^nRu?{XjGE;H$U?$#q{+^<)3Px(Zv|w^KwxDtGGr`&;_J+c53G8%<8d!d zjgd~{kNSmsvu`6szUOq9G9!nUO(q^|5e-}$Mu>MD3pD~?*ZC3Yhs2cx6owDgw`ZtK z+L(6*tL-`G0B6IaX6NSKJiKR=n3O$}NJr%!52ge*>LWD-vd^GY&m!ZisV zI^0+6eQS!uSaz;Bb1k+;H#@2V*Vk~UmRqK7ZRo{?B>~9(`8Fe^P;Yeat$2~9nILM% z^jBisyFPacAIs9?9nMXSaxRFGY2sb2eR7C{iba5ck#y;>be)-Q zxym>&`Fz9r&*6&k6(MfuxDzO2dBPH80a0TjBZr>rWBiC*L32`Z>1VKP0zuVIotdI; z_oE{91?~qgSMbmtm1qR&%g5}u?9mjjw1CnYjX)g$++E)xI1Q>erN_GIc-kGNg)jl*wyF6 z#g7rRX*TA5{x|9jbQieSAGf*v<2V1et?4)4UKe}t8$kEhpGH_U%yIOqSK}BW92)<{ zoN&IH9zAYBB%!2+?u8}PdvMh`cjLWa?CrzdCa*X|Mmr@i)GKS?40LR z8a`!s5>>B~Uo$hSWUMa#dr5SQko!wO?$6J9=(3UtwM`{ij%=M|9#ho9K$ZsPOP7x4 zI9^tTC9Lg(=jfClSAuhoE662>wHpM(Bz#Y7TwGji?1>U zOMB_aIICvevaoD^HEm`Q`-|9zA^Dr(pcL2>IffT{UO6@e6GXzu-qSGNskZN{0)$!2 z8vmP8h;neS6sXJY*0D^{$Na3JL_~w&jPiIqHrtPT(jc$GRr>9vB>v}};^L_~ff@N} z^zMD}Q6c}M8vW;I7{A%5|9<9>ztAU4ae!&0h+S302wn?=5UMExT6e3YQSv~tF)*M3 zHf*O|puwdB8v%Cy8<+qsp>b&R1;O4(wubI|5>cR`Q7wuF_eGtFSouwVd{0Bv9k#n5 zc@1ed3n}?#nA}nmh2mBPu;mf9f3W0yc30?r@fNT!Z0C64C(V_m-6~FqO){Y>99mZ7 z7`?lpgSbtsHp{HhFZ(6)SLyTI)XHLdx7u243L`PhV_@C$)2h1(~#;dc7z~%33PBLIs@v+-bFv}%E#!JX*fy4RwInwo<><{Wv*0Q zMG_?-H;4dP_X|qipI8M^zf4AFSgA@BAF;xW_fOiScC zZ4VTLKR-)mVpl?gFX|-hzbyolM?d8^uhzZ0qPx4A8mLx8j!u6(m*MtqT4E!oRh;(WAj$f`1E)i|Kl`<`3aXdnS9MCkw8xdW2pl zr8Kp5DO9LaYhHc7{vhIV!1?X|Tb$jjeN056ZV0*xf z8y0B-bcVMh6eKU!5cPR$k3(9Zv+zK{ zrcjogom`Uv@7Qx-kscXyD{%WVU#EVV``>;yqv!&h7d6*E(los=3ml?8hDZ|B^&UnL z^7y$<@^`}~fjwgJ^CJIe@l!K-K*0>*7bP!-hle{lI(qXkrXV9Y0--0lg%rR5gT@@m zWU`|lHl=@y1Hu@74>SA1w=5#+pF)%qSC9ycH=a|aFo1fOq>VX2Pw?duuNq|(QPBelEK8M zgQ%x}zdWT%UjC;RV-=nt0wV# zNBDwbIU+l{RS}+ANIDveWsF2P6=RNA_tXtXZ8FW!p(wBz8)o#PEf#YIIqbWD!D)wEPqRTaz&$GFH+ z8ACHiXnZz{XCMlsi)4gA5b?SBO8iAa7E_OBIC}fCdP}l?c@aB~BJcUeLkcNp{7f-> zMou<{!W2cl8ILsYL#TwucQlwa@gm{sSxw7a5lQ7)U)ZzpCr;Lg7QjMPmS`}`<5Sd= z*UWo#1t;bXM_mry-*#Qxoa8<(tzv}rtW*X!^ZI#eY9hO;Dj6wAeRe>Pw@!bWdsQgK zjG4GO+pLPXJy17RnWM=bDABW{IJH*ZvHO(Qz!M2PaTg22hPFkFQBppll0nw44D|uu*ySnj_pP3^N4!W%~JY4V`2)r;{z=+T7J%J)J`}=g8d>PDCdQ7 zgBuw1WcrmRRZS-g`6fnlzfjGH*bKXyUH)7HNv zt_e77_;cfPYf6Qu?#p`;jzpV@yN0PWMU(x3lq+NXhRdg*M%5de8BEs|D7X<8YocW? zx;-{QKH?3lBYb|6-63z4h20`Pz4Mfg7y|aJs2tEQ<#D&z2g90{B0ddva0ND=w?BgE zuHKDGNNn;KJ~-=eM2xez$YW@-844E_l?mf70xXGkO>jV>mmuz^ov>1aIk)fW!>F8w z0Hh{imvPvTey!M+3}pdZ4@}6m2k{ck=V*bkCMt-NB@bB3_q3$d{D@GlHnqd$CF zq`hqV(sHn(DAA9K*oHUYERgfNkrDq2hJS7UdZ6F|KA1CHmNJK!CZVx1PDHhOBYltD z)deN+le1CU;u{&g#c^Q$ImV$jz|h^o^YU4Ta#w^U1}*1uxjcj1Sk1=Ayo`j2SF$k< z>A-b!Ar-iremCqedK>Bei7AkLd3@$Pu>dI?)2e9+Qh}a_{QPN=y)JGu04RUj?RAmu zFK$PF4mSLI&%+|y|JT?O%Xp;ctBCtQ)i-*~SgnW@%6ltz9hO?N=~!jTgF78{+G8>) zR~nO6A9Bq>zdBa)@aETRg(_6Mg>vLKL(UT!cM_Nmb zH_BKpWlyj8?gga>P#i(N2x2v2TG&o~(sB+bRv51l+v^zRBxSLdY@OAcY4?t`L+nve zjQK$Wr=pi;QE9+zHkB68)qkApGtYHHjRa*q3~2cli%nE_h_? zlkLw@=$l6$&$!~(C2xf5?kr$ma4_#;SR`)L51kf@+vB`3W0Z%+7T4CkLfWVH1$AQ3 z!jkn`$j@T%qg+)yfInZJihWagL40#!?scPn*`&CppGWBbd`2s$$o$9V&&*(E#i~$<6l=B{^|9;D5CupM6{{#7X3ofR4h(MCb?-~CKbe>=z9_F z!YJp;i0@}mS~g}^Zn@CsuTU*RC)qrW@Wr%icI3viiuw0+A*|-!BNgq>;A5&ZD0%Tf z=D~U0(3;2t&hxO;vX|k6!sTnIl8vVi$Lq^T0!S-Z{D{E5^E+vMUa9O{bWnbp&C{U1 zkD2<15&r=93Rf=e*=W*!$4ngBK{zRDuFCqpQ|$NLSvt>2nutcb1{qF&W67V-{fZd= zHGA(W6$a1nozv=EO;iL5{IX{sW!ONJVkkjO?h+%WXjdGPA`UA!&h~Ewtfo z)%rZokPoyhdns2x*}Va(`JJc5RV9vw50Th&FZlD{JkC$6Bv(EM*CfC*npN0T^eztV z$`h4v1#Bxr&t>vr-&Q;&Q@xqYOlz5Y_|(HtM$(_*I(jkFK)MFFSo^J#*2{;3BW6v6Acn^Xw;@qE*xQ_t zu>pPeN#^0;{)z!wnvS|+%kjjGw*9nQIy^@38FNDB1|^WJ9IPWlrck6r{u$PRDdW`! zZJAkHaOlwrPVS~3B3-nrE)GX%T84Id-NlvhS_3-g&7%_fX@O1ut0u~?9#;1paJj94 zt3fMp*meFjc*NeN*4C)c>lfU9Sn~s#IIn&s3Zb9Yv$9Tum380W)>0oHs82Sxv zzlM$Z_>Q&I{AXq-K`wXOoZEztv0R7M)QjS5?SN&KBzQCWf^_6ka$Z9a=R`;N#@21s zMZC|KLwJE#GccB>ROa|EFv4eyj8F)jBJ5f*onK@v+Go90K$g!|k8t5Pwn?1J2DPqt zm@t_#iK4yxAQRc^9AtPv))z{-LbZD3sEUM8%QG_xbY89l$04&!kGVR@lG|#HkyJh2f^E z6rFBr(Ph^M%&y<^a& z2!zT3DOSo2rDhl3Zw0SRrmBx3*i$Xi&scWcJ)-w@4O@j zFChD5m8b+sqXI`iCO^;Qp!YVISR&&vQ5-hb^~fJPuVr6`xV(Ujy5}X0;+_e&fCMMa zD=T+`)jzbu!su@$@s#1jrpa>}ukmhX_ayAo5F#x z1q#f*KY%aul^;!yF=0iId2CJ>4CV1?Q>7~9zxB}kRY#Ucf2e_G+4d?|hVu&a>LE9zZ-2St- z`0wV4`+}B1)Qc)Jda^P1+Y0y5--94(?eC6^7q{A&T3|PA0YghkSJ7P~8}lVQ41zMw zgx%KByHbPmZ1shp%~uuC>oM6h?cMnX+I=->5UfwiTzU9_e$&81dNhwS9-V6j=X-oO zJ;o6coTp`P-i?~i(&>g0L`Jux-Iv*r{IMcOQxd)H1DDJV55jAJ5QItq3;;dHb?}uK z^QD}+@FAl&wf}LKEh}+Z$7h)iXcfTc?2i((2HUOfN&$DrvW^7d{XqCNE3amU$5ul- zd;h)~qULp%j|hYz>!vMZnMo`Ndb87_r6C#|aoC@R*$Df}3CVr-V0;6|MemB6p1isK Rasr0OF%zpJMTgwK`%hJ~pv(XO literal 0 HcmV?d00001 diff --git a/pygmt/tests/test_clib.py b/pygmt/tests/test_clib.py index bd76643c712..89cb7770424 100644 --- a/pygmt/tests/test_clib.py +++ b/pygmt/tests/test_clib.py @@ -474,7 +474,7 @@ def test_virtual_file(): data = np.arange(shape[0] * shape[1], dtype=dtype).reshape(shape) lib.put_matrix(dataset, matrix=data) # Add the dataset to a virtual file and pass it along to gmt info - vfargs = (family, geometry, "GMT_IN", dataset) + vfargs = (family, geometry, "GMT_IN|GMT_IS_REFERENCE", dataset) with lib.open_virtual_file(*vfargs) as vfile: with GMTTempFile() as outfile: lib.call_module("info", "{} ->{}".format(vfile, outfile.name)) @@ -491,7 +491,12 @@ def test_virtual_file_fails(): Check that opening and closing virtual files raises an exception for non-zero return codes """ - vfargs = ("GMT_IS_DATASET|GMT_VIA_MATRIX", "GMT_IS_POINT", "GMT_IN", None) + vfargs = ( + "GMT_IS_DATASET|GMT_VIA_MATRIX", + "GMT_IS_POINT", + "GMT_IN|GMT_IS_REFERENCE", + None, + ) # Mock Open_VirtualFile to test the status check when entering the context. # If the exception is raised, the code won't get to the closing of the diff --git a/pygmt/tests/test_plot.py b/pygmt/tests/test_plot.py index e1f842b4642..623593cf5fe 100644 --- a/pygmt/tests/test_plot.py +++ b/pygmt/tests/test_plot.py @@ -258,6 +258,23 @@ def test_plot_vectors(): return fig +@pytest.mark.mpl_image_compare +def test_plot_lines_with_arrows(): + """Plot lines with arrows. + + The test is slightly different from test_plot_vectors(). + Here the vectors are plotted as lines, with arrows at the end. + + The test also check if the API crashes. + See https://github.com/GenericMappingTools/pygmt/issues/406. + """ + fig = Figure() + fig.basemap(region=[-2, 2, -2, 2], frame=True) + fig.plot(x=[-1.0, -1.0], y=[-1.0, 1.0], pen="1p,black+ve0.2c") + fig.plot(x=[1.0, 1.0], y=[-1.0, 1.0], pen="1p,black+ve0.2c") + return fig + + @pytest.mark.mpl_image_compare def test_plot_scalar_xy(): "Plot symbols given scalar x, y coordinates"