[Implemented] Signatures for plotting functions #833
Replies: 13 comments 11 replies
-
I favor a I would propose a superclass of To create a I would e.g. create my default configuration and the use it every time I create a figure. |
Beta Was this translation helpful? Give feedback.
-
@Yuri05, @msevestre Any thoughts? |
Beta Was this translation helpful? Give feedback.
-
Mental model for me to work off of (thanks to MS): |
Beta Was this translation helpful? Give feedback.
-
List of parameters we want to expose to the users (might be extended): All plots:
IndividualTimeProfile:
PopulationTimeProfile:
SimulatedVsObs:xUnit = yUnit ResidualsVsTime:
Multipannel combination (grid)
|
Beta Was this translation helpful? Give feedback.
-
After discussing the current implementation with @pchelle, I have realized two important things:
The only ones that can't be specified in the theme are the following: title = NULL,
subtitle = NULL,
xlabel = NULL,
ylabel = NULL,
legendTitle = NULL,
xAxisTicks = NULL,
xAxisTickLabels = NULL,
yAxisTicks = NULL,
yAxisTickLabels = NULL, Out of these, I would argue that it's not necessary for users to have the ability to change the number of ticks on the axes and their labels. So, if we discount them, we are just left with the following: title = NULL,
subtitle = NULL,
xlabel = NULL,
ylabel = NULL,
legendTitle = NULL, So now we go back to the original question: Which of these is better? Option-1Most of the other parameters are specified by the theme: plotXYZ <- function(dataCombined,
title = NULL,
subtitle = NULL,
xLabel = NULL,
yLabel = NULL,
legendTitle = NULL,
xUnit = NULL,
yUnit = NULL,
plotSpecificParameter = NULL,
tlfTheme = NULL) {
...
} Option-2Most of the other parameters are specified by the theme, and the ones that aren't specified by the theme are specified in plotXYZ <- function(dataCombined,
plotConfiguration,
xUnit = NULL,
yUnit = NULL,
plotSpecificParameter = NULL,
tlfTheme = NULL) {
...
} Option-3Most of the parameters are specified by the plotXYZ <- function(dataCombined,
plotConfiguration,
xUnit = NULL,
yUnit = NULL,
plotSpecificParameter = NULL) {
...
} In my view, the Option 1 now becomes the most appealing because users don't need to learn - and we don't need to create and maintain - a whole new class just to specify the plot labels. But they will need to learn how to change the theme, if they want to change the look of the plot. What do you think, @msevestre, @Yuri05, @PavelBal, @StephanSchaller? |
Beta Was this translation helpful? Give feedback.
-
I would stick to implementing I don't see a problem to "learn" about this new class (as it is actually very simple for the user). |
Beta Was this translation helpful? Give feedback.
-
I agree with Pavel, would also prefer having the PlotConfig as parameter rather than the tlfTheme. Additionally, we could provide a possibility to create a plot Configuration from a theme And maybe even something like P.S. what does the argument |
Beta Was this translation helpful? Give feedback.
-
No, as discussed, we want to have a Units are also definitely a part of PlotConfiguration. |
Beta Was this translation helpful? Give feedback.
-
Hmm, that was not what I have in mind. For me, a theme is a set of defaults. So I would prefer to
Then the easiest way to call the plot function would be to use ALL the defaults, e.g. If I need some plot config settings different from the default:
|
Beta Was this translation helpful? Give feedback.
-
I don't understand this. Why do we have parameters that we cannot set ? Or where those parameters that you thought existed in tlf but are not directly accessible? |
Beta Was this translation helpful? Give feedback.
-
Wouldn't it be more consistent with the rest of the code to default to |
Beta Was this translation helpful? Give feedback.
-
Output with the defaults look good now (cf. #871 (comment)): plotIndividualTimeProfile(myDataCombinedObject) h/t @pchelle for chaperoning this. |
Beta Was this translation helpful? Give feedback.
-
Related to #674
As noted in the relevant issue, all plotting functions should support the following optional arguments:
This means that the plotting function signatures will be bloated compared to other functions. Is this an issue?
Option-1
Bloated signature but simpler for the users since they won't need to know about the underlying R6 objects from tlf.
Option-2
An alternative would be to have a simpler signature. Here, we would expect the users to enter PlotConfiguration object that contains most of these needed details.
But this means that the users will need to know all about the following plot configuration classes:
BoxWhiskerPlotConfiguration, DDIRatioPlotConfiguration, HistogramPlotConfiguration, ObsVsPredPlotConfiguration, PKRatioPlotConfiguration, TimeProfilePlotConfiguration, TornadoPlotConfiguration
And also about the classes that constructors for these classes themselves rely upon:
BackgroundConfiguration
,Scaling
,xAxis
,XAxisConfiguration
,YAxisConfiguration
, etc.That's a big price to pay (for the users) just so that we can have a simpler signature!
In other words, there's no such thing as a free lunch.
What should we be doing here?
Beta Was this translation helpful? Give feedback.
All reactions