Location of MapScatter in map_scatter.py is wrong [Bug]
Closed this issue · 15 comments
File diagnostics/map_scatter.py has the wrong location for emcpy.plots.map_plots.MapScatter. Instead it uses emcpy.plots.plots.MapScatter which fails.
To Reproduce
Run eva testIodaObsSpaceAmsuaN19.yaml. It will fail in eva/diagnostics/map_gridded.py at line 18 when if tries to create a plotobj using emcpy.plots.plots.MapGridded.
Expected behavior
Test case should run to completion.
Additional information (optional)
I'll be glad to fix this if I can be assigned to it.
Changing the location to emcpy.plots.map_plots.MapScatter works -- it looks like emcpy changed at some point and eva didn't get updated. Unless there's a reason to import all of map_plots I propose importing only emcpy/plots/map_plots/MapScatter in map_gridded.py and not using the explicit path to MapScatter in the code.
Thanks for finding and being willing to fix this @EdwardSafford-NOAA
@danholdaway
I'm running into a situation I don't understand. In diagnostics/map_scatter.py the problem is simply that emcpy.plots.plots needs to be changed to emcpy.plots.map_plots. Making that change allows the test case to run to completion. But if I change the import line to
from emcpy.plots.map_plots import MapScatter
and then try to use MapScatter directly
self.plotobj = MapScatter(latvar, lonvar, datavar)
the result is a seemingly unrelated error codition:
Traceback (most recent call last):
File "/home/Edward.Safford/.local/bin/eva", line 8, in <module>
sys.exit(main())
File "/home/Edward.Safford/.local/lib/python3.9/site-packages/eva/eva_base.py", line 226, in main
eva(config_file)
File "/home/Edward.Safford/.local/lib/python3.9/site-packages/eva/eva_base.py", line 202, in eva
eva_figure_object.execute(data_collections, timing)
File "/home/Edward.Safford/.local/lib/python3.9/site-packages/eva/plot_tools/figure_driver.py", line 95, in execute
self.make_figure(figure_conf_fill, plots_conf_fill,
File "/home/Edward.Safford/.local/lib/python3.9/site-packages/eva/plot_tools/figure_driver.py", line 127, in make_figure
layer_list.append(layer_class(layer, self.logger, data_collections).plotobj)
File "/home/Edward.Safford/.local/lib/python3.9/site-packages/eva/diagnostics/map_scatter.py", line 33, in __init__
self.plotobj = MapScatter(latvar, lonvar, datavar)
File "/home/Edward.Safford/.local/lib/python3.9/site-packages/eva/diagnostics/map_scatter.py", line 14, in __init__
if 'channel' in config['data']:
IndexError: only integers, slices (:), ellipsis (...), numpy.newaxis (None) and integer or boolean arrays are valid indices
I don't understand why making the change to import only MapScatter in map_scatter.py results in the MapScatter object not being able to correctly parse config[data]. Any thoughts on what I'm missing? If not I'm going to just issue a PR for the simple rename of plots to map_plots and move on; this has already taken too long!
@kevindougherty-noaa any thoughts on this?
I think this is the issue:
Line 44 in cb5a15f
I think that's it but I'm not sure I yet understand what I'm seeing. I had had that line commented out for reasons I'm not able to recall. I have a local copy of emcpy installed with eva. Once I uncommented that line and rebuilt eva the test case runs to completion with the change to the import line.
I do think the plots->map_plots needs to happen, but you say that causes another issue?
No that change is fine. When I tried changing the import line from
import emcpy.plots.map_plots to from emcpy.plots.map_plots import MapScatter that's when I ran into problems. I just uncommented lines 43-4 in eva/setup.py and both cases now work.
@EdwardSafford-NOAA @CoryMartin-NOAA I believe at one point the plots.map_plots were in the plots.plots but I am not sure if that is the issue you are referring to. Also, the emcpy version in setup.py should definitely be updated to a more recent stable branch.
I think there are two places this needs to be changed, for mapScatter and mapGridded. This, plus the updated EMCpy hash. Sorry @EdwardSafford-NOAA that you had to struggle with this
@CoryMartin-NOAA yes I see the same issue with mapGridded. I'll add that to this issue.
I checked the latest version of emcpy develop is dcdc868e7f48bb1ff55507f34c0548742af8bec1 . Is that good to use in setup.py?
it should be but we need to test to be sure
Should I wait for that testing and include an update to eva/setup.py in a PR for this issue, or just issue a PR without updating setup.py? Alternately, is the necessary emcpy testing something I can do?
emcpy should be good unchanged, but I think if you update the MapScatter you also have to update the setup.py. That would explain why the eva tests don't fail in the CI (they're still using an old version of EMCpy). I can help with this tomorrow if you'd like.
Ok, that would be great. Thanks!
Closed by PR #79