From 2fa24c2f910e583e40cf5418fd25c3244a327542 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 04:08:02 +0000 Subject: [PATCH] 14 Three small fixes: kill zone selections, placement files, and the -g bts view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Builds on PR 8 (#24).** It doesn't depend on PRs 10–13. [This PR's changes on their own](https://github.com/adamf/vida/compare/pr8-web-viewer...pr14-small-fixes). ## Three small fixes: kill zone selections, placement files, and the `-g bts` view None of these change a normal run. They only matter for input files with mistakes in them, or for the one graphics view that crashed. All the characterization recordings match except `graphics_outputs`, where the `-g bts` stage now makes its files instead of crashing. ### 1. A kill zone's `selection` missing a part crashed (`vevents.py`) `readZoneSelection` checked `("attribute" and "logic" and "value") in theSelectionDict`. In Python, `("attribute" and "logic" and "value")` is just `"value"`, so only `value` was checked. A selection without `attribute` or `logic` then crashed with a `KeyError`, instead of printing the warning and ignoring the selection, which is what the code meant to do. Each of the three is now checked. ### 2. A placement file with bad lines could lose good ones (`vplacement.py`) `checkSeedPlacementList` deleted bad lines from the list while looping over it. That skipped the line after each deleted one, so: - a bad line straight after another bad one was kept; - a later bad line could make it delete the wrong line. It now keeps the good lines in a new list instead. ### 3. `-g bts` crashed (`vgraphics.py`) The combined bottom + top + side view filled the side view's template, which takes six numbers, with only four. It now gets six, the same way the other side views get theirs. The `.cfdg` files it makes are now pinned by the `graphics_outputs` scenario. I haven't seen the picture itself (the tests don't run the `cfdg` program), and that view's layout code looks unfinished, so it's worth a look. ### Tests - **Unit tests:** - a selection missing each of its three parts is ignored (the old code crashes on two of them); - a placement file with several bad lines, two of them in a row, keeps exactly the good ones. This replaces the test that pinned the old behaviour. - **Characterization:** `graphics_outputs` re-recorded. Its first two stages are identical to before; the third now makes files instead of crashing. The test README's list of bugs found is updated. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_013425y5x98TqAv3BvHodpdx --- Vida_Data/vevents.py | 4 +++- Vida_Data/vgraphics.py | 4 +++- Vida_Data/vplacement.py | 10 +++++++--- tests/characterization/README.md | 9 +++++---- .../golden/graphics_outputs.json.xz | Bin 8728 -> 9540 bytes .../scenarios/graphics_outputs.yml | 7 ++++--- tests/unit/test_events.py | 9 +++++++++ tests/unit/test_placement.py | 11 ++++++----- 8 files changed, 37 insertions(+), 17 deletions(-) diff --git a/Vida_Data/vevents.py b/Vida_Data/vevents.py index 60041da..0650edf 100644 --- a/Vida_Data/vevents.py +++ b/Vida_Data/vevents.py @@ -125,7 +125,9 @@ def readZoneSelection(theDict): if "selection" in theDict: theSelectionDict=theDict['selection'][0] #make sure the selection array has all three needed elements - if not ("attribute" and "logic" and "value") in theSelectionDict: + #(each one checked: ("attribute" and "logic" and "value") is just + #"value", so only that one used to be checked) + if not ("attribute" in theSelectionDict and "logic" in theSelectionDict and "value" in theSelectionDict): print("\n***WARNING: Selection array needs exactly three elements: attribute, logic, and value") print("\n***Ignoring selection criteria") zoneSelAttribute = "none" diff --git a/Vida_Data/vgraphics.py b/Vida_Data/vgraphics.py index 6e508d2..75d3ebb 100644 --- a/Vida_Data/vgraphics.py +++ b/Vida_Data/vgraphics.py @@ -187,7 +187,9 @@ def initCFDGText(theGarden, displayType, percentTimeStamp, maxHeightPlant): worldTypeRule=worldTypeRule+CFDGTopWorldRule worldTypeRule=worldTypeRule % (worldX+(worldX*0.3), worldY1+(worldY1*0.3), worldX, worldY1, worldX-1, worldY1-1) worldTypeRule=worldTypeRule+CFDGSideWorldRule - worldTypeRule=worldTypeRule % (worldX, worldY2, worldX-1, worldY2-1) + ###the side view's rule takes six numbers, as in the other side views + ###(it used to be given four, which crashed) + worldTypeRule=worldTypeRule % (worldX+(worldX*0.3), worldY2+(worldY1*0.3), worldX, worldY2, worldX-1, worldY2-1) if percentTimeStamp>0.0: timeStampCode= CFDGTimeStamp diff --git a/Vida_Data/vplacement.py b/Vida_Data/vplacement.py index b3fd768..8633b88 100644 --- a/Vida_Data/vplacement.py +++ b/Vida_Data/vplacement.py @@ -47,15 +47,19 @@ def checkSeedPlacementList(seedPlacementList): aLine = [correctType(x) for x in aLine] seedPlacementList[i]=aLine i=i+1 - i=0 + ###keep the lines that are a species name, x, y and a whole number of + ###cycles. (This used to delete the bad lines from the list while going + ###through it, which skipped the line after each one it deleted.) + goodLines=[] printErrorMessage=0 for aLine in seedPlacementList: if type(aLine[3])==float: aLine[3]=int(aLine[3]) if (type(aLine[0])!=str) or (type(aLine[1])!=float) or (type(aLine[2])!=float) or (type(aLine[3])!=int): printErrorMessage=1 - del seedPlacementList[i] - i=i+1 + else: + goodLines.append(aLine) + seedPlacementList=goodLines if printErrorMessage: print("***Improper seeding file format...") print(" Questionable lines will be ignored.") diff --git a/tests/characterization/README.md b/tests/characterization/README.md index f58e887..22314a2 100644 --- a/tests/characterization/README.md +++ b/tests/characterization/README.md @@ -180,10 +180,11 @@ been fixed since, and each now has a scenario that uses the feature. a `TypeError` on Python 3.12+, and on 3.11 it was already a `ValueError` for worlds of odd size. -Still open: the combined bottom + top + side view (`-g bts`) crashes with -`TypeError: not enough arguments for format string`, because -`initCFDGText` in `vgraphics.py` fills the side-view template, which needs -six numbers, with only four. The `graphics_outputs` scenario pins this. +5. Fixed: the combined bottom + top + side view (`-g bts`) crashed with + `TypeError: not enough arguments for format string`, because + `initCFDGText` in `vgraphics.py` filled the side-view template, which + needs six numbers, with only four. The `graphics_outputs` scenario now + makes that view too. Also noticed, but not crashes: in `disperseSeed`, the terrain binary search calls `elevationFromPixel(thePixelValue)` without `theGarden.maxElevation`, diff --git a/tests/characterization/golden/graphics_outputs.json.xz b/tests/characterization/golden/graphics_outputs.json.xz index abc92668d490a5fb886f1c58b09d3c342f92e47d..2fd17d67be65ade1c55f9510f6e1cfbd328c884d 100644 GIT binary patch delta 3702 zcmV-+4vF!YM8ryv7zN|cXe9x$9o`myh99t842c`-%MmvFQ_%HVgv9qRiTXcssri>1 zJ*kORH50%*&yEl7%$1tb&%L6Z)`aZ!QVHu9m+U%F>j`AYs?js2roAXfJ`CHsOsxh$89TrcAzMCmtC!$*8snHuisr!%5NEN%r~}IlLdYZeHR$0=wfFIewK4_ zC|J}8<=J}gC$JNe)BkiBiDYS5mh;j<9~zWB0CEg7g{Zy8(hPR$zw=Xn=fMSx(!S5n z9}@d?I-P)v+)e4j=l~3*etn>P^NhX7Hh_i~CZ8s3Wm+~B2)v1`hh2__M7uh=yHWxT z_o6b+lF%<39m7^S8gh0|b9YleT67&Dph^9xDmvIi{Yb=}Je1c_5KjVux~SAP-3_Wc zG>*c+<>urt<*JTsiRpBIJChi!ZqFV$Biy4ycs%JozMTT(vJ+gg7qTkWcayNLP@Vx{ z7+YOSJD%L7)`~lYEz644XeeA8jm2g`4)ZS4=O2~+MKj@5XoWAyNKynyHhv_wPp!u6 zgxFFqR&5(uUY?4d;-|&o2jlkZbYC+YXjKRt@G09n?z~)zBpZU>{E>V8+aHYOLGdb<->U(yGgS`!B|zZI3& z0U6@<@zQ7t*2Z{SWHkKRexYFQrvMD5hcF`HoGgSHX3sC?!%yQK0lc2SP@C<4a09$SUJ8HmK0XRlzXq8D z<+Rmiw47a-DQuTZiu?P{l)=U!ucdXzg{Ww=B z$yI79iP&uIqo;mdn|Xz5jL-py`fVcXkjAQ#cNFV|=^kTD=Op|!vM7U_NzfWe)zd#~ zwEv|JW$|!-*)4S}b0%=bDy0uHxe->|gCyqAG0_Mshn6j$BMI3QDTJ__ir!6=MmaYC z5e(c97$T8GUR{6G?C~MaCDLQ$q6I*yPff!Sz3#z4NBiHu{E8NdgYtX>2afI`_H>ZI z)<xi)#|M8^sKb#sXdB(oh58ZNS7kL=N>&ASr z66q$b(RlZo+!W_=;OD?$C-j$LCM5qBLc3oqiB0V9z$qS|jv&c}H-En-$~{`I`Tki? zsx`HL9S&wXG7IV#4gwCCZy{E&y7vkt9q8RT-hJ$M-IOK5X^=&iXx!x~E(T;z@G-7p z+AH-sg)aG;TWbmAc=y{+>7C`Ro>o7lt1PM|dj_4nXa*%}R(CJgBDAx5P2EDLXGH{O zxm41o3TZ%M64T$i3%#NC_8yYhWNa3jrM4)4>a;2o#>}M$uv=OoUzQ65+ige#X9+6H<$h8^ulGo3Xkv~zijVa>~AYb4SJ2g=#FzJb?6ZEhwDvL)v~A9g>A zBqMe)T;^d8_PTuqKzMkTF|r=^G+86dN_=1^y0(ptj9HQJ$7oM@AHzbOp8xwXJ-h3F zxKEG5x1vZ?bIh)h-*BndVPVtj_a7g5HxjvY`c!t!p`m=NmlFwA6;tt^J4ilK?K3r? zEd#h)0e=X2S};Ok$MiB;>5?zI=KS^WkT1~ zg6hm6A`vf~>IgEX!!^%&u2c#Dvm=#%3wo@j8bGw2S!d*aXkPq{ms^}gH`vN8-$nck z$+1o$(bQAw%>}TJ7rUiBU38|kbNga*JG(k<^hj0=Yf)<}(3yM2nH|*7&9^^A@>6rD z1}(iP|BW+uVC7G7LhjSyb9Scj%OwyaFj#3+ah%?Utz?Gs;HrB+e3dz6zg!S{VRs&s@@(s(%Xrs$bDKoPF zUVbi~VkrZ3`K?EGOcSumuwml%QyZ&fNB9kjstQFvcu#keuK!UP6)qA55iik+p7E5P ziO$Vq1TXYhL#uR!D4%5Ds14_T?V$?_^C?#@v{xaA(HMTd45wxw;SWjky-5JIpWP9ejU0saExSZWmBmffjxbiNcPc-*>^&>htR z4~uo0O&w~U*9~LU$;MGrjpVJDZCzt+vJNcauQ$IWZH*k4DTUo7Q?VVC+1OX(dY#q< zLBj-C;2P+K^i9lv3}-nRRccDwbk_P9R}1$zzeG~8JdI7NoWlEi_qXw}3bxR_jxkGb zeQfWXjAD&Bh_-0gPcGmapyi!d-mAu6D6uv*hB;d={%Fbt-tOwu%M&Unl6ucLTOQy# zW%km!SA9Hg-1m3BV}SqRj6zUDubqLfo8?H}ZO4KrJ*<0w8CFcvTM;V;y?A{~E>(59 z2$Ih&1?(#c8~%64Q~ooOQf*a3<;-j0v?3GS;saKpXaeny`ya$aUA7UB>|h&!OMMDv zuM`)TZ_s}qx}qVh z4HMm5%T<3>C&~KoslGN}D=^@EG7ERN@K}q5A4DpD{uxRq(4z@=z;Z>aN8+{>)QI$J zs_Q@LJtkQyK{!rrRtRSlwJ0nqexNjaj7`cG=o1SUKtfF?Gr+4wR#j^_V8vbCqdSkL z=?H`JTf9 zf$O7k`CSsA-VvzG3Hq*+1YvHX%$H#5Ov>qsRpgc6W$5~}JUIh9*cVf95lS5;?}&QS zCGe1&N)0FIsPUByaj#K*y54bmEY{XGXEp{6w3OY#n zjXALExI%0}fO+N27|HPB7Lw*i*?#X;>FhBn43;DcxY`Nr%|B6JR9@cAtXF{QoNYv# zouKb60G?q5c$AdLXhxHr(6Hs?*;jEr9*66F_TJ!yqxse8i+VJ*NwLbxbW~COSuq)Z zvaIZ)bSJQo{=&mD-s;|WYQ(qsrRKP+FfOv{&6uQSzJvK7iit&j^C)rD@1xW+g+$YJ zZi$R(3uJySra;vQ(LqM7$hc_EUf|Vz7_qb6C`8f9IP>lA~uCX zhq@DD71%dwZsaoh?chPKuUwp4yEZa^+?9e|Bn{=UyPI?f%Ke7dUmuwrK_A$oel3XB zj|%jy5K9W$S`jl&3&&_MW@|beqdI#X@eF|8FYONvJS0vRA^ZEFm%%VC@<(zQ=}i9W zX?Bwo76Jv~YpKzDp=*8Ye@CFf=tC9O>|t}Av**%JIwYEZ9#kfamSyqEnBgsd)3&f5 z2(2bXt$Y3RGA#u0fpkM53Ic_H7eQor)Eu8;@Q8{E^1T!kzdte;S{uYZG|w&*`1`?g zdr!Z7dm?+RLxKSj5LTiQp#T{UefHEN?sX!4X8h$7L_d;xL8L!DF;1zgmuc?o8WxGU z`gkLIC7&dJT@T#g1%6ob?bvT+1f0M#U@>EmZs89@_JzSkTkG1V63hmQazo7_jPAUV zI;>Dvcy`2jJ)Im5Ta$=c@mFy|@Af?;8Y3Za`_6k_000000Bkel`E5_&VE_T0O6i^t U052AdtFgpq`vL#}000D8T8I@BssI20 delta 2884 zcmV-K3%m5hN|;2D7zN_nu_4v59o`mygPnL9>Fw>yuMxe>sFGzSv4PJ)$VA1aJP{1E3~?U2nB8|IS1}tx*U=!+J`RyUfr}G?(pvV|nc4 zYu$qh#mB)l>>D=2Ztcwy{bZUjR48Mt_689ADq3?0(;9j{?_u^H*!s$`bs#0;X0jh? zYT*G@hDKa9Sk5?q10K{L0GZHr?Uq7duKZ%Zl5gYtx$~LgvT@!miGt}&^P#dihKmJd z28D19s$fIE%Q@1ku`%82$7#HOm-nofpab`*D&@2RLJrW2TUt#z)FCkwj^tw^0LgAV zXiZn#KCVF;c)L?a)L+=tS=ojrx1DvZPZNB|Mkn=V>>Ds_k1!L&XIEN9O2Ksq+8O8I z&Td5j+#tJuiU|VmY6t=stB4|r9C>EZ{53ivN_7mQ0bxEH75wVLN$(7QuGMykg*BK% z=Z7m^cMGvLq#E5TsnnoTvC=AA^=M7mbb1h?%&*>*_YUYgnJX=8W)GPeep3ei-1gu6 z8G2!niKz5HBxU(Pp~LRVkDKfYn++`x*qa-fmsXLGLv79b(yWVyR$TkKlSn%S4oYZt zYI;QG>qiZm+CqW0uf~~w5VkUDTdbPBYuXw3XNgn#*`29^dUOm~wFtQKzwGf#>3)}m zeNjv)Q6bnf2G>IUMDfy!oy_M5GaZ0s*#@!o)f?nrut^q~kb^D`FGniCE-7(iU5X#Eqc4ba`yZ{u37k@7 zr|e8>HRvHjC@E`yr`BtYS*fP&k1w{n*J=a#2~b2o`jHjbA7Iz9Dm$Mgc)`5;SP=PH zAW;Wd*9X3vj=XPn^CIog#GgU7q5@`6{x$ZkxH>KTg4$-REMwN*w_-;hdb8VlSkB;n zaFgK$If*jjP&`F|^|mc>g&r5ANmHy%K$Y*_mnzf>BxmJY5%a5~3_OexCN0xD9yuzepp00ETB-&Bzuk$2*dnJWtzO*Db~j zwTd@(*R^Em0aRMzWpN8ItUwAyM|Zb>F67$ZRg}31SWD{h+4Ewx(oh7>4^GR`E?V!( zvq@g0ue$3qh3YYjP&p5am-+!wclgb9bF0x6%d~)h-UVO=@OJlnfu+BI6#&#rs9pm9 zRG5FzVDYKmoo~L^KqmEu0D6S3Wzi(dSh7{Wm)NKr`!v&;!A$_BtosE1I)Q3*TCY}4 zZQ{39>i&oZx5i@Zmc`hZdtMLsSa|~3MOp3THKTONQ4RC?G)`t#k{FdoMFA5an#m^9bD(i~MIo^QD{)16bIp#TGpp$P`Nw%l&g9(*D+}k~dAmmJH8* zT+wY81{M7mGM|B}ol&0TT_FFwwcmqYq#nNN4{!D;o*}{>s4JLTkC4U^Kbn7Cnqh~3 zX3yDoXxrk04q7bzN%Y?H7UR9DS-gW5n+yP2D#q4b zwFMKnVmH#g6d3Ve){M7Zy`Mx8?BJ#Dy-5^8piXJ(d8{yEJ&DRx*u%18e?<%MCKgs$ z?!5FG(Tj1FTJ;blZFjiJYr<%--SM`6*34qnf&F6m^uOc`R}d0Lvgjs?r?VvA$}Uq1 zfdqTztFwiy`y+%?n?C0tyG=Sa(W$`$NTJgDfzg#}0uU(D#dF_c{b+Px{G!n42uN8g z-FH$ZX|JFRBnDvModc~i971mGQ)>!6QLc+jlwo+~P}@`nW0p{_}PR4&=+ zCIAIKAp@(2_Z7mQCXrBh*kdankvj?a2Wyo&=ShKt#c{-24f=k}^nM;Yw{9*EJl6YK zW?2Q96@CyeM@jRY`P&}C@dCUiVK3#>jOU@Z&N`Yp2#dVbYBzjNW4kc2mtn;l;4j5? znuMh%^x2>#_4TV!U?8~pR9|&}Ov@ARmICNHfuf)Eh!idS7KUMBz+S_{RK`0ssH$o8 z-F*a+qzu{yv`sW_xg3`9EUaIzmBpMHQ*AQI$4Foy-}gLdusv>H{lD2(^F#w=8Ns;Rz)^LMQ(1_Asjii>KG>vL zT4?1Ui#wR5zskIe0eVM4 z#L=$;ZU-cWROhyun6Uu~s>K}um#7s2!A&vWOofvw;2Cg+1oI3ez=%1*y==Q%kbiA) z)$L6W?2qLHKi@<3TBrEwOuWyq&q*Le=R2=7U%8xLg~$E@2WjJfdfD9R47)$LeYc0h zkt@Rwo@?Q>3`p_sJW7M0N!CKEkHOr|)e2W_BqQN@wvlTgE+NCf20)-bQdz0Fm4!|; z>7`bh1igG-JSntx{^dUdtuQB%AVca!+Ff+SBnZQ}^SpC2gZEhwI+qA`--b5yo|@>- zabwu}apiCf^}fM>LzWooC&EGB;kE?zgaO|Jl9cz{$yzH$)@LO)pL(0+e^;GMIg_BB;9M^^vs`lXo@ z%F5)5d&!VWmnG~sb{xx_1S=nY6gn`66^8bxsCplc5;Cz%<5V<$8q#DVu%Wk2I4MJFC60R820^- zASk<^lIin*mNE1@0%DK+!1*%4Lt0=WWWJ^9p`hTnh{|+6)w4L`_xTVTGfURgh{%&E zYjrciL*FH_KqG=frF9>Wx>ZbUQ)t~<(Zh(dxo@b;joZ;0z9fr#1fvQ+R#C?hEnN9e#Bd>`7B>Edzd6NibUvrOQ|Ep{a1DX00025 iWg%f9VBVbo0r5k!w+jHbi&mDg#Ao{g000001X)@e9iATm diff --git a/tests/characterization/scenarios/graphics_outputs.yml b/tests/characterization/scenarios/graphics_outputs.yml index 2bb975e..02b0d20 100644 --- a/tests/characterization/scenarios/graphics_outputs.yml +++ b/tests/characterization/scenarios/graphics_outputs.yml @@ -2,9 +2,10 @@ description: > Graphics output: the ContextFree (.cfdg) files and the 3D .dxf files, kept on disk (-c) so their names and contents are pinned. Stage 1 makes the combined bottom-up + side view and 3D; stage 2 makes every other view - except bottom + top + side (-g bts), which currently crashes (stage 3): - initCFDGText fills the side view template, which needs six numbers, with - only four. The external cfdg and ffmpeg programs are not run. + except bottom + top + side (-g bts), which is stage 3. (That view used to + crash: initCFDGText filled the side view template, which needs six + numbers, with only four.) The external cfdg and ffmpeg programs are not + run. species: [species/generic] stages: - rngstart: 808 diff --git a/tests/unit/test_events.py b/tests/unit/test_events.py index 7913520..3529039 100644 --- a/tests/unit/test_events.py +++ b/tests/unit/test_events.py @@ -44,6 +44,15 @@ def test_a_zone_without_a_selection_selects_everything(): assert vevents.readZoneSelection({})[0] == "none" +def test_a_selection_missing_a_part_is_ignored(): + # Each of attribute, logic and value is needed. Only value used to be + # checked, so a selection without attribute or logic crashed. + for missing in ["attribute", "logic", "value"]: + selection = {"attribute": "heightStem", "logic": ">", "value": 2.0} + del selection[missing] + assert vevents.readZoneSelection({"selection": [selection]})[0] == "none" + + def test_a_selection_with_bad_logic_is_ignored(): zone = {"selection": [{"attribute": "heightStem", "logic": "!=", "value": 2.0}]} assert vevents.readZoneSelection(zone)[0] == "none" diff --git a/tests/unit/test_placement.py b/tests/unit/test_placement.py index 018244e..38d4bd1 100644 --- a/tests/unit/test_placement.py +++ b/tests/unit/test_placement.py @@ -23,8 +23,9 @@ def test_whole_number_coordinates_are_rejected(): assert lines == [["random", 1.0, 2.0, 0]] -def test_after_a_bad_line_the_next_line_is_not_checked(): - # The check deletes bad lines from the list while looping over it, so the - # line after a deleted one is skipped: here the second bad line is kept. - lines = vplacement.checkSeedPlacementList(["a.yml, 1, 2.0, 0", "b.yml, 3, 4.0, 0", "c.yml, 5.0, 6.0, 0"]) - assert lines == [["b.yml", 3, 4.0, 0], ["c.yml", 5.0, 6.0, 0]] +def test_every_bad_line_is_left_out_and_every_good_line_kept(): + # This used to delete bad lines from the list while looping over it, which + # skipped the line after each deleted one: the second bad line was kept. + lines = vplacement.checkSeedPlacementList( + ["a.yml, 1, 2.0, 0", "b.yml, 3, 4.0, 0", "c.yml, 5.0, 6.0, 0", "d.yml, 7, 8.0, 0", "e.yml, 9.0, 1.0, 2"]) + assert lines == [["c.yml", 5.0, 6.0, 0], ["e.yml", 9.0, 1.0, 2]]