Several plotting.py functions don't work - keep or ditch?

[freemansw1]

As noted by @snilsn in #155, several of the functions in plotting.py do not currently work. His notebook is here: https://github.com/snilsn/tobac_functions/blob/master/plotting_functions.ipynb which contains a list and description of several of the broken functions.

As I have mentioned a few times before, I'm skeptical of the utility of most of the plotting functions in the first place; they come from the time when tobac was designed for use in just one paper rather than the more broadly used tool that it is today. I am inclined to immediately deprecate the broken functions, but I think this should be a more broad community discussion. The plotting functions will only become harder to maintain when we finish the Iris/Pandas -> xarray transition slated for v2.0, as many of them will need to be individually updated.

For now, my suggestion is only to depreciate the broken functions (which I list below), but I do think we need to consider trimming these functions down more broadly as we start to think about the v2 transition.

Known broken functions (all in tobac.plotting):

• plot_mask_cell_track_follow
• plot_mask_cell_individual_follow
• plot_mask_cell_track_static
• plot_mask_cell_individual_static
• plot_mask_cell_track_2D3Dstatic
• plot_mask_cell_track_3Dstatic
• plot_mask_cell_individual_track_3Dstatic
• plot_mask_cell_track_static_timeseries

[mheikenfeld]

Sorry to see that so many of the functions were left in here in a broken state... As Sean said, most of the code in the plotting functions has been developed for an initial paper using tobac, but then the code base of the rest of the package seems to have moved on without updating the plotting functions accordingly (and without tests that has not been flagged up). I think I did actually move on to not really use these functions any more at some point and switched to more customized plotting functions functions myself, so I doubt anyone has really made explicit use of them at all in the last 2 years or so.

So my suggestion would be to maybe even deprecate the entire plotting bit v2 and rather cover that in a bunch of notebooks.

At the very least we should move all the functions out that outputs "png" or "pdf", seems like all of these are not really the right thing to keep as part of the package itself but rather something to make available to people as building blocks for vizualisation in the form of notebooks/documentation etc.

Maybe a couple of the functions could be useful in visualizing things on a very high level in tutorials for people starting out with tobac (such as plotting all tracks as a kind of Spaghetti plot and so on) or some usefull things could make it into the utils?

For whatever we keep, we should make sure we have it covered by simple tests, maybe based on the general test case we already have....

[freemansw1]

Thanks for your insights, Max. I agree with your assessment and think that some basic functions (like map_tracks) should stay as they provide an easy entry point.

[snilsn]

Thank you, @freemansw1, for raising this issue, and thank you, @mheikenfeld, for your thoughts.

I just want to add here that I don't know if all the functions I've marked are actually broken. I found specific issues for some of them, but others I could have just tried with the wrong inputs. What I can say for sure, however, is that these functions are not intuitive to use. Troubleshooting and improving usability would mean a lot of work, and it's probably not worth the effort. I'm in favor of at least deprecating the functions I've marked as broken.

Other functions (like map_tracks)are used in the example notebooks, I would like to keep those, since they offer simple out-of-the-box way to plot results without many lines of code. Maybe we can move those to the utils at some point, like Max suggested.

As a side-note: Many of the plotting functions have a non-optional 'cog' argument, which I assume should be the output of centerofgravity.py, but this module is probably worth a similar discussion.

[freemansw1]

This has been resolved for v1.5.0 in #200