Return map potential data from Map Plots #305
Conversation
|
Why change the "plot_mapdata" function ( pydarn/pydarn/plotting/maps.py Line 67 in 0e459a3 pydarn/pydarn/plotting/maps.py Line 455 in aabbb70 |
There was a problem hiding this comment.
Looks good, before we wrote this in pyDARN I basically did the same thing for our real-time display to print out the potential contours so I think it's good to return everything making up the plot by amending the plot_mapdata method rather than just the calculate_fitted_velocities method. Then the user can use the data to do whatever they want, as some of this data isn't give straight from the map file (I think that's what you mean in the comment above @bharatreddy as the calculate_fitted_velocities method just finds the velocities wrt the spherical harmonic functions and doesn't contain any other map data?)
I guess also should we consider renaming plot_mapdata? It was a hangover from the beginning of this file and I don't know if it's as user friendly as just plot_map? It's pretty normal or standard to return all the data used to plot the plot in a call to a plot, so it would be expected to return the data with it.
Anyway, I just saw a few things that I thought I should comment and now I've ran out of time so I'll do a proper test soon!
| ret_dict['imf_info']['by'] = by | ||
| ret_dict['imf_info']['bz'] = bz | ||
| ret_dict['imf_info']['bt'] = bt | ||
| ret_dict['imf_info']['delay'] = delay | ||
|
|
||
| return mlats, mlons, v_mag |
There was a problem hiding this comment.
I've not tested yet, just looking at the code. Should we return ret_dict here? And include the mlats, mlons and v_mag for the point on the map in the ret_dict too?
| ret_dict['fit_pot']['mlats'] = mlat_p | ||
| ret_dict['fit_pot']['mlons'] = mlon_p |
There was a problem hiding this comment.
Maybe keep these in the ret_dict with the '_p' so they don't get mixed up with the mlats and mlons from the vectors?
| mlat_p, mlon_p, pot_arr = cls.plot_potential_contours(fit_coefficient, lat_min, date, ax, | ||
| lat_shift=lat_shift, lon_shift=lon_shift, | ||
| fit_order=fit_order, hemisphere=hemisphere, | ||
| **kwargs) |
There was a problem hiding this comment.
Flake8 info: These are >80 in length. PEP8 style to make sure the lines are 79 chars or less. I can do a proper flake8 sweep and amend things later if there might be some other changes.
|
I think #343 might help with this, is there any data that you want from any of the functions specifically? I think I got the array of potentials, hmb postion and the contours (along with the data) all in the return. |
|
Let me know if #343 (now merged into develop and in the new release branch) addressed what you might need from the map plotting functions, I'm going to make this a draft for now so no one merges it by accident, if it is addressed we can close this PR! |
Map potential data
Return fitted potential / velocity / e-field datasets from Map potential data plots. This enhancement will help to share the fitted datasets for event analysis in various conjunction studies using SuperDARN. @bharatreddy @Shirling-VT @carleyjmartin
Approval
Number of approvals: 2 (testing and code review where possible)
Test