travistai2 commented 10 months ago

I went through some of the plotting code and just realized that the package now depends on ggplot2, which is loaded whenever I use load_all() or library(pacea). I also noticed that some of the plotting functions use ggplot, e.g. plot.pacea_buoy. I have a some questions regarding ggplot and plotting of the data.

Are we all in for having ggplot2 as a dependency? I would have written my plot functions differently if that were the case, which would have also helped in plotting multiple panels for geospatial figures. If so, this also changes the style of how I would write the vignettes.
Generally, are the plots for users to visually/qualitatively view the data? Or are they for the users to manipulate/customize? Shouldn't this be consistent in our plot functions?

seananderson commented 10 months ago

At the most I’d expect it to be in imports. Same with the stats package.

andrew-edwards commented 10 months ago

Thought we were okay with ggplot being a dependency #14. Even if I wasn't at first ;) And, yes, I'd used it in some of my plotting functions.

I'd say the plots are for users to initially view the data, and understand what's there, but can also be customisable for them. I'd hope people will do things we haven't thought of. But giving something to start with is good.

travistai2 commented 10 months ago

14 - right, I forgot about this. I'm all for using ggplot to create quick plots, it makes plotting the spatial stuff easier. But I think it is probably best practice to avoid it as a dependency and have it under 'suggests' or 'imports'.

andrew-edwards commented 10 months ago

I probably wasn't using the term dependency in relation to its strict R definition! So whatever works.

pbs-assess / pacea

General plotting of different datasets #39

14 - right, I forgot about this. I'm all for using ggplot to create quick plots, it makes plotting the spatial stuff easier. But I think it is probably best practice to avoid it as a dependency and have it under 'suggests' or 'imports'.