Fix NameError when writing grid text in certain cases #70
Conversation
|
Ok, so now I understand, this is a flaw in pycoast in my opinion. @lobsiger I'm curious what your thoughts are. The reason the I won't be fixing this in this PR, but it is pretty annoying that I feel like could be added. For example, check all the edges, if no edges then put labels on the ends of the lines. |
|
@mraspaud @pnuu I think I'd like to keep the original test and my new test (which is a copy of it with one keyword difference) as two separate tests knowing that the results should change in a future PR. I can refactor them more if needed. 🤔 Or maybe I should parametrize them with pytest and in the future I just have two versions of the end result. |
|
FYI I avoided doing stuff I should be doing and tried rewriting some of the important code in pycoast to use more numpy. It ended up being slower...so probably won't be wasting time doing that for a while. |
|
@djhoese I entered your PR 70 manually and get no grid labels anymore. This explains IMHO why your test passed with "write_text=False" or without "write_text=True". Did I miss something? Here is an improved script to test. It uses s = scale and t = translation. As published it should produce my latest image on the google list that I still made with PyCoast 1.5.0. |
|
@djhoese sorry Dave. I DID actually miss something. I have played yesterday with lon_placement, lat_placement and these are still in my code empty. So everything works as before. I propose to let it as is with labels only at the sides of the area_extent[]. |
|
@lobsiger I agree on the test passing because there are no labels being written/drawn. Unfortunate that some a "basic" use case doesn't have labels. I'm not sure I understand your second comment. With this PR, is everything as expected? |
|
@djhoese as you can see in the script I attaches above I have: cw.add_grid(img, area_def, (10., 10.), (2., 2.), lat_placement='', lon_placement='', write_text=write_text, outline='white') This is left from yesterday when I wanted to test "write_text=True" but without label placements. It did not work with PyCoast 1.6.0. Now today I patched my PyCoast 1.6.0 according to your [PR 70] and tested with the above script again and was confused not to see any labels (at first). Only one hour later I remembered the placement and noted it's still empty. I deleted this like cw.add_grid(img, area_def, (10., 10.), (2., 2.), write_text=write_text, outline='white') and everything worked as expected. Default grid label placement is lon_placement='b', lat_placement='lr' IIRC. |
|
FYI version 1.6.1 is available on PyPI and was just merged on conda-forge. Should be available for download from conda-forge in the next hour or so. |
As described on the mailing list, somewhere in the refactor of #63 I introduced a bug. As can be seen in the code the iteration variable
index_arrayis used after the for loop, but if the iteration never happens (empty iterator) then this variable isn't defined. This PR fixes this issue with the least disruptive way I could think of. In my refactor I had made things into generators so doing a simplelenon theindex_arraysisn't possible.More worrying is that this copied test should have completely different results (right @lobsiger?) since it has
write_text=True, but the same expected result.pngcompares successfully.TODO:
git diff origin/main **/*py | flake8 --diff