Add plot configuration class and time profile plots #871

IndrajeetPatil · 2022-04-11T11:57:11Z

This PR is coming from a branch that doesn't have issue number because it was supposed to be short-lived and was off shoot from #858, but it has now become the primary branch and #858 has been closed in favour of the current one.

TODO:

Part of #674

Related to #815
Related to #832
Related to #833

Closes #877
Closes #878
Closes #920

> “I became insane, with long intervals of horrible sanity.” - Edgar Allan Poe

this makes ospPlotConfig more general purpose class

Yuri05 · 2022-05-10T15:28:52Z

I wonder how this (still incomplete) picture would look like in black-white? 😄

But in fact I asked myself already 20 years ago: why would somebody print papers/reports in BW?

IndrajeetPatil · 2022-05-10T15:41:55Z

I wonder how this (still incomplete) picture would look like in black-white? 😄

I know this is a rhetorical question, but here you go nonetheless 😅

IndrajeetPatil · 2022-05-10T15:43:47Z

Can this be resolved?

@StephanSchaller Good catch. Yes, this is easy to resolve; I will take care of it.

Yuri05 · 2022-05-10T15:49:32Z

I know this is a rhetorical question, but here you go nonetheless 😅

Nice. As a color-blind person I cannot even handle the coloured version of it, so the BW-version - well... 😄

msevestre · 2022-05-10T17:41:50Z

Argument: If you print a report/manuscript on paper and print in B&W (what many (most?) people still do)

That would be interested to see if this is the case. I would have implicitly said otherwise, without any data to back my claim.

That being said, the fact that some people may still print their report, and when they do, print in BW, should have no bearing on how we create our default plots.

At any rate, it should be very easy to create a plot that do not overload grouping criteria such as line types + colors, when this is logically not required

StephanSchaller · 2022-05-10T20:11:18Z

At any rate, it should be very easy to create a plot that do not overload grouping criteria such as line types + colors, when this is logically not required

Yes, as I understand it now, everyone will be able to set their preferences so that everyone will be happy :-)

PavelBal

Some changes to the documentation are required.

Also some code refactoring resp. questions.

R/default-plot-configuration.R

PavelBal · 2022-05-11T09:48:29Z

R/default-plot-configuration.R

+#' R6 class defining the configuration for `{ospsuite}` plots. It holds values
+#' for all relevant plot properties.
+#'
+#' Note that the values this objects contains are of general-purpose utility. In


Please remove this, it is confusing as it sounds as these values can be directly used in e.g. "plot()" function.

R/default-plot-configuration.R

PavelBal · 2022-05-11T09:51:00Z

R/default-plot-configuration.R

+#' - font face: `tlf::FontFaces`
+#' - linetype: `tlf::Linetypes`
+#'
+#' The available units for `x`-and `y`-axes variables depend on the dimensions


"Variables" - don't understand in this context.

"Available units for x and y axes depend on the dimensions of data sets to be plotted".

Changed accordingly.

PavelBal · 2022-05-11T10:11:52Z

R/plot-population-time-profile.R

+  # plot -----------------------------
+
+  # Which dataset types are present?
+  datasetTypePresent <- .extractPresentDatasetTypes(dataCombined)


From here on, same comments as for the "individual" function.

Lots of code duplication, I think most of it can be packed in an internal function. Actually only few lines of code differ.

PavelBal · 2022-05-11T10:13:09Z

R/utilities-plotting.R

+  dataTypeUnique <- unique(dataCombined$groupMap$dataType)
+
+  if (length(dataTypeUnique) == 2L && all(dataTypeUnique %in% c("observed", "simulated"))) {
+    datasetTypePresent <- .presentDataTypes$Both


Return early

PavelBal · 2022-05-11T10:13:16Z

R/utilities-plotting.R

+  if (length(dataTypeUnique) == 2L && all(dataTypeUnique %in% c("observed", "simulated"))) {
+    datasetTypePresent <- .presentDataTypes$Both
+  } else if (length(dataTypeUnique) == 1L && dataTypeUnique == "observed") {
+    datasetTypePresent <- .presentDataTypes$Observed


PavelBal · 2022-05-11T10:13:29Z

R/utilities-plotting.R

+    datasetTypePresent <- .presentDataTypes$Both
+  } else if (length(dataTypeUnique) == 1L && dataTypeUnique == "observed") {
+    datasetTypePresent <- .presentDataTypes$Observed
+  } else if (length(dataTypeUnique) == 1L && dataTypeUnique == "simulated") {


Last else if not required if you return early.

PavelBal · 2022-05-11T10:18:54Z

R/default-plot-configuration.R

+#' @field pointsColor,pointsShape,pointsSize,pointsAlpha A selection key or values for choice of color, fill, shape, size, linetype, alpha, respectively, for points.
+#' @field ribbonsFill,ribbonsSize,ribbonsLinetype,ribbonsAlpha A selection key or values for choice of color, fill, shape, size, linetype, alpha, respectively, for ribbons.
+#' @field errorbarsSize,errorbarsLinetype,errorbarsAlpha A selection key or values for choice of color, fill, shape, size, linetype, alpha, respectively, for errorbars.
+#' @field plotSaveFileName,plotSaveFileFormat,plotSaveFileWidth,plotSaveFileHeight,plotSaveFileDimensionUnits,plotSaveFileDpi File name (without extension) format to which the plot needs to be saved, and the specifications for saving the plot.


Also - how to change between output to a file and ouput to standard device (e.g. "plot" tab in RStudio)?

Yes, this is now mentioned in the new Saving plot section.

PavelBal · 2022-05-11T10:20:12Z

I also wonder - why is it that slow? Creating a simple individual time profile takes like two seconds, the same plot in esqlabsR is generated almost instantaneously. Is it ggplot, or TLF, or..?

IndrajeetPatil · 2022-05-11T13:05:25Z

Here are what the default plots look like.

I am not too happy with the fourth color and shape (yellow with downward triangle), but that can be changed in tlf itself and the plots will adjust accordingly.

Individual

Population

PavelBal · 2022-05-11T15:05:19Z

Last refactoring:

plotIndividualTimeProfile and plotPopulationTimeProfile are 95% identical.

Create one internal function. There are only two sections that differ between individual and population, and they are identified by checking whether quantiles is NULL

.plotTimeProfile <- function(dataCombined,
                                      defaultPlotConfiguration = NULL,
                                      quantiles = NULL) {
  # validation -----------------------------
  
  validateIsOfType(defaultPlotConfiguration, "DefaultPlotConfiguration", nullAllowed = TRUE)
  defaultPlotConfiguration <- defaultPlotConfiguration %||% DefaultPlotConfiguration$new()
  validateIsOfType(dataCombined, "DataCombined")
  validateIsSameLength(objectCount(dataCombined), 1L) # only single instance is allowed

  # data frames -----------------------------
  
  df <- dataCombined$toDataFrame()
  
  # Getting all units on the same scale
  df <- .unitConverter(df, defaultPlotConfiguration$xUnit, defaultPlotConfiguration$yUnit)
  
  # Datasets which haven't been assigned to any group will be plotted as a group
  # on its own. That is, the `group` column entries for them will be their names.
  df <- .addMissingGroupings(df)
  
  # `TimeProfilePlotConfiguration` object -----------------------------
  
  # Create an instance of `TimeProfilePlotConfiguration` class by doing a
  # one-to-one mapping of internal plot configuration object's public fields
  populationTimeProfilePlotConfiguration <- .convertGeneralToSpecificPlotConfiguration(
    data = df,
    specificPlotConfiguration = tlf::TimeProfilePlotConfiguration$new(),
    generalPlotConfiguration = defaultPlotConfiguration
  )
  
  # plot -----------------------------
  
  obsData <- as.data.frame(dplyr::filter(df, dataType == "observed"))
  
  if (nrow(obsData) == 0) {
    obsData <- NULL
  }
  
  simData <- as.data.frame(dplyr::filter(df, dataType == "simulated"))
  
  if (nrow(simData) == 0) {
    simData <- NULL
  } else  if (!is.null(quantiles)) {
    # Extract aggregated simulated data
    simData <- as.data.frame(.extractAggregatedSimulatedData(simData, quantiles))
  }
  
  if (!is.null(quantiles)) {
    dataMapping <- tlf::TimeProfileDataMapping$new(
      x = "xValues",
      y = "yValuesCentral",
      ymin = "yValuesLower",
      ymax = "yValuesHigher",
      group = "group"
    )
  } else {
    dataMapping <- tlf::TimeProfileDataMapping$new(
      x = "xValues",
      y = "yValues",
      group = "group"
    )
  }
  
  profilePlot <- tlf::plotTimeProfile(
    data = simData,
    dataMapping = dataMapping,
    observedData = obsData,
    observedDataMapping = tlf::ObservedDataMapping$new(
      x = "xValues",
      y = "yValues",
      group = "group",
      error = "yErrorValues"
    ),
    plotConfiguration = populationTimeProfilePlotConfiguration
  )
  
  # Extract current mappings in the legend (which are going to be incorrect).
  legendCaptionData <- tlf::getLegendCaption(profilePlot)
  
  # Update the legend data frame to have the correct mappings.
  newLegendCaptionData <- .updateLegendCaptionData(legendCaptionData, populationTimeProfilePlotConfiguration)
  
  # Update plot legend using this new data frame.
  profilePlot <- tlf::updateTimeProfileLegend(profilePlot, caption = newLegendCaptionData)
  
  return(profilePlot)
}

The exposed functions just forward to the internal:

plotIndividualTimeProfile <- function(dataCombined,
                                      defaultPlotConfiguration = NULL) {
  
  .plotTimeProfile(dataCombined, defaultPlotConfiguration)
}

plotPopulationTimeProfile <- function(dataCombined,
                                      defaultPlotConfiguration = NULL,
                                      quantiles = c(0.05, 0.5, 0.95)) {
  # validation -----------------------------
  
  validateIsNumeric(quantiles, nullAllowed = FALSE)
  validateIsOfLength(quantiles, 3L)
  
  .plotTimeProfile(dataCombined, defaultPlotConfiguration, quantiles)
}

Co-Authored-By: Pavel Balazki <[email protected]>

IndrajeetPatil · 2022-05-11T15:28:40Z

Great idea! Done.

msevestre · 2022-05-11T16:06:57Z

I did not have time to review the code at all. I will comment on this later today or tomorrow

PavelBal · 2022-05-11T17:06:10Z

I did not have time to review the code at all. I will comment on this later today or tomorrow

Sorry, this has been handing here for so long that I thought you already looked into it.

IndrajeetPatil added 26 commits April 4, 2022 13:23

adds first drafts for plot config function + class

37e2197

> “I became insane, with long intervals of horrible sanity.” - Edgar Allan Poe

more aesthetics

8de281b

cover more properties

4461041

remaining not necessary, or derivative of existing

ad233e4

docs

5a3799a

Merge branch 'develop' into 833_create_plot_configuration

447925b

docs

2909f7c

fixes

cff38fd

Merge branch 'develop' into 833_create_plot_configuration

fffa94d

Merge branch 'develop' into 833_create_plot_configuration

7ad6df6

remove unused parameters

77d070c

Update DESCRIPTION

b023daf

minor

39b2452

use LabelConfiguration for labels

2d7affc

add background configuration

f6ce26e

add an intermediary class

9d02de9

this makes ospPlotConfig more general purpose class

Update plotIndividualTimeProfile.R

b7bf095

update

d85b48b

remove createPlotConfiguration()

ae3f161

use enums in docs

6e71242

choose defaults

680a6a4

Merge branch 'develop' into 833_create_plot_configuration

9f9b5b0

bump version of ospsuite.utils

c4fc0cf

minor

ddec561

Merge branch 'develop' into 833_create_plot_configuration

28535a9

defaults for export

43230c6

IndrajeetPatil marked this pull request as draft April 11, 2022 11:57

Merge branch 'develop' into old_plot_config

229b9d9

This comment was marked as outdated.

Sign in to view

remove constructor; add aesthetics for annotations

983dd05

PavelBal requested changes May 11, 2022

View reviewed changes

PavelBal mentioned this pull request May 11, 2022

Make esqlabsR theme for default plot configuration esqLABS/esqlabsR#225

Closed

IndrajeetPatil added 8 commits May 11, 2022 12:57

closes #926

0c213b7

docs

3689730

More docs

a2471a1

section for scaling

00f054f

set tlf data to NULL when dataType isn't present

8769641

also update docs

84893cc

Update plot-population-time-profile.R

872da4f

add visual regression test for population profile

f672de5

IndrajeetPatil requested review from msevestre and PavelBal May 11, 2022 12:56

IndrajeetPatil added 2 commits May 11, 2022 15:12

also test that changing linetypes works

1c85939

more docs

f0a732f

extract out common plotting function

8e052b9

Co-Authored-By: Pavel Balazki <[email protected]>

PavelBal approved these changes May 11, 2022

View reviewed changes

PavelBal merged commit 94d6f6a into develop May 11, 2022

PavelBal deleted the old_plot_config branch May 11, 2022 15:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add plot configuration class and time profile plots #871

Add plot configuration class and time profile plots #871

IndrajeetPatil commented Apr 11, 2022 •

edited

Loading

This comment was marked as outdated.

Yuri05 commented May 10, 2022 •

edited

Loading

IndrajeetPatil commented May 10, 2022

IndrajeetPatil commented May 10, 2022

Yuri05 commented May 10, 2022

msevestre commented May 10, 2022

StephanSchaller commented May 10, 2022

PavelBal left a comment

PavelBal May 11, 2022

IndrajeetPatil May 11, 2022

PavelBal May 11, 2022

IndrajeetPatil May 11, 2022

PavelBal May 11, 2022

PavelBal May 11, 2022

PavelBal May 11, 2022

PavelBal May 11, 2022

PavelBal May 11, 2022

IndrajeetPatil May 11, 2022

PavelBal commented May 11, 2022

IndrajeetPatil commented May 11, 2022

PavelBal commented May 11, 2022

IndrajeetPatil commented May 11, 2022

msevestre commented May 11, 2022

PavelBal commented May 11, 2022

Add plot configuration class and time profile plots #871

Add plot configuration class and time profile plots #871

Conversation

IndrajeetPatil commented Apr 11, 2022 • edited Loading

This comment was marked as outdated.

Yuri05 commented May 10, 2022 • edited Loading

IndrajeetPatil commented May 10, 2022

IndrajeetPatil commented May 10, 2022

Yuri05 commented May 10, 2022

msevestre commented May 10, 2022

StephanSchaller commented May 10, 2022

PavelBal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PavelBal commented May 11, 2022

IndrajeetPatil commented May 11, 2022

Individual

Population

PavelBal commented May 11, 2022

IndrajeetPatil commented May 11, 2022

msevestre commented May 11, 2022

PavelBal commented May 11, 2022

IndrajeetPatil commented Apr 11, 2022 •

edited

Loading

Yuri05 commented May 10, 2022 •

edited

Loading