pfmc-assessments / nwfscSurvey

Tool to pull and process NWFSC West Coast groundfish survey data for use in PFMC groundfish stock assessments
http://pfmc-assessments.github.io/nwfscSurvey/
10 stars 8 forks source link

Move towards using more {ggplot2} graphics #34

Open ericward-noaa opened 3 years ago

ericward-noaa commented 3 years ago

Right now things are a mix of base plots and ggplot -- it'd be good to pick one and update functions to be consistent

chantelwetzel-noaa commented 3 years ago

You will have to pull my base plots from my cold dead hands. I think the ggplots are only being used to create map figures since this is the one function that appeared to be easier with ggplots. I will look into way to move these plots over to a base version. Thank you for looking through the code and documenting places that we can and should make improvements.

iantaylor-NOAA commented 3 years ago

I can move the map plot to base graphics of that would be helpful, as I didn't find the ggplot version that much better (and it led to the install issue noted in #32).

@ericward-noaa thanks for your help with this package in general. Can you clarify the benefits of sticking with just base or ggplot?

ericward-noaa commented 3 years ago

@chantelwetzel-noaa Hah!

@iantaylor-NOAA Good question - I think it's largely aesthetics, and what you're using the plots for (maps, etc). I think in terms of the functions and parameters being passed in, a lot of the functions using baseplots have a bunch of arguments (xlim, ylim, titles in some cases). Switching to ggplot might be a pain, but the functions wouldn't need those arguments. Instead, I think the function would be something like

f <- plotBioStrata.fn(...)

And then 'f' would be a ggplot object that you could later manipulate, and save other places:

pdf("plot.pdf") f + ggtitle("Main title") + xlim(0,10) + scale_color_viridis() dev.off()

Or whatever. I guess a downside of the ggplot graphics would be that figures might be tweaked slightly in different assessments, and could look more different than with the base plot equivalents (where users have little control)

iantaylor-NOAA commented 3 years ago

Thanks @ericward-noaa. I appreciate the explanation and the value of internally consistent controls. We should get the package passing the checks and better documented, but it doesn't sound like there's any rush to get the base vs ggplot consistency worked out. We added dependency on ggplot2 to the much larger r4ss package which has allowed contributions from folks who prefer that framework, but with 400,000+ lines of code, much of it plot-related, there is very little likelihood of converting everything to ggplot before the whole package is rendered obsolete by superior replacements.

The challenge we have in the FRAM PEP team is that very few of us have skill with ggplot whereas many of us have been using base graphics for 20 years.

I see users tweaking ggplot figures to suit their needs or aesthetics as an upside, not a downside. But even if we could wave a magic wand to make the switch to ggplot graphics, I think that the developers and many of users of the packages in nwfsc-assess, who aren't yet familiar with that framework, would have LESS control of the graphics due to lack of familiarity with how those controls work.

chantelwetzel-noaa commented 1 year ago

@kellijohnson-NOAA has dragged me kicking and screaming into using ggplots for a number of new plotting functions. The current approach has been to create new functions while retaining the older base R plotting functions to maintain functionality for people's pre-existing workflows. The new plotting functions also have a new naming structure where the will start with plot_ to better indicate the purpose of the function. One example of the new plotting functions is plot_comps (will eventually replace plotFreqData.fn).