Seurat: Consistency of do.plot and do.return among plotting functions

[b-dawes]

Some of the plotting functions in Seurat have do.plot and do.return parameters which control whether a plot is drawn on the current device and whether a ggplot is returned, but there is not much consistency between different functions.

For reference, I went through some of the functions and wrote up what they return below.

DoHeatmap:
Always returns a ggplot object. Has an option for do.plot set to true by default.

DotPlotand FeaturePlot:
Have an option for do.return set to false by default. Always generate a plot.

FeatureHeatmapand DimPlot:
Have an option for do.return set to false by default. Generate a plot only if do.return is false.

JackStrawPlotand DimElbowPlot:
Always return a ggplot object. Never generate a plot.

MetageneBicorPlot:
Returns a ggplot object unless return.mat is true. Always generates a plot.

VlnPlotand RidgePlot:
Have an option for do.return set to false by default. Also have an option return.plotlist set to false by default which changes the return value from one ggplot object to a list of objects. Generate a plot only if do.return is false.

Out of these different options I think the options for FeatureHeatmap and DimPlot make the most sense to me as asking for the ggplot object will generally mean that you want to do some further manipulations on the object before plotting.

[lazappi]

I have noticed this as well and it would be great if there was some consistency. My strong preference would be for plotting functions to return a ggplot object by default, without any calls to print etc. That way if you want to adjust the plot you can capture the output in a variable (e.g. plot <- FeaturePlot(...)) but if you don't it will be displayed. I think this is probably what FeatureHeatmap and DimPlot are doing. I find the functions that always show the plot particularly annoying as they force me into workarounds when I don't want that to happen.

[andrewwbutler]

Agreed. There are a few places where we need to tighten up consistency across functions and this is definitely one of them. In this case, I agree with @lazappi that returning a ggplot object by default seems like the best way to go.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[jspaezp]

@mojaveazure @lazappi @andrewwbutler

Sorry to resurrect the dead thread, but I feel like this enhancement would highly improve the usability of the package. Let me know what branch I should submit the pull request to.

Looking at the code it seems like it would not be too much work but we should definitely discuss how backward-compatible we would like the changes to be. I suggest an additional argument for the arguments that return a list at the moment, such as return.list = do.return which could be removed in further revisions.

[andrewwbutler]

Hi,

We agree and worked to implement this in v3.0.0. In general, all plotting functions should now return the ggplot object by default. The exceptions here are anytime we have to combine plots, we return the results of the CombinePlots function call (providing combine = FALSE will return the list of ggplot objects). The other exception that I can think of is DimHeatmap which by default uses image to generate the plot instead of the ggplot alternative (geom_tile or geom_raster). We chose to deviate in this case because image was significantly faster to render to the screen and we see the primary utility of this function for quick exploratory visualization (though there are parameters to allow for plotting via ggplot as well).

[jspaezp]

Hello,

I went through the code of the current master branch and it does seem fixed (I am also glad many explicit print statements were removed). Definitely should have checked the last version before re-opening the thread.

Amazing job!
Thank you very much for the quick response and the amazing project