Open-Systems-Pharmacology / TLF-Library

TLF Library implementation in R
https://www.open-systems-pharmacology.org/TLF-Library/
Other
9 stars 6 forks source link

Changing `ospsuite`'s `linesColor` palette modifies identity and foldDistance lines colors. #472

Closed Felixmil closed 11 months ago

Felixmil commented 1 year ago

In ospsuite, when the linesColor palette is changed, the foldDistance and identity lines also change:

devtools::load_all()
#> ℹ Loading ospsuite
#> Loading required package: rClr
#> 
#> Loading the dynamic library for Microsoft .NET runtime...
#> Loaded Common Language Runtime version 4.0.30319.42000

simFilePath <- system.file("extdata", "Aciclovir.pkml", package = "ospsuite")
sim <- loadSimulation(simFilePath)

simResults <- runSimulation(sim)

obsData <- lapply(
  c("ObsDataAciclovir_1.pkml", "ObsDataAciclovir_2.pkml", "ObsDataAciclovir_3.pkml"),
  function(x) loadDataSetFromPKML(system.file("extdata", x, package = "ospsuite"))
)

names(obsData) <- lapply(obsData, function(x) x$name)

outputPaths <- "Organism|PeripheralVenousBlood|Aciclovir|Plasma (Peripheral Venous Blood)"
myDataCombined <- DataCombined$new()

# Add simulated results
myDataCombined$addSimulationResults(
  simulationResults = simResults,
  quantitiesOrPaths = outputPaths,
  groups = "Aciclovir PVB"
)

# Add observed data set
myDataCombined$addDataSets(obsData$`Vergin 1995.Iv`, groups = "Aciclovir PVB")

DPC <- DefaultPlotConfiguration$new()

DPC$linesColor = "red"
# DPC$pointsColor = "green"

plotObservedVsSimulated(myDataCombined, foldDistance = 2, defaultPlotConfiguration = DPC)

Created on 2023-06-29 with reprex v2.0.2

Felixmil commented 1 year ago

TLDR:

ospsuite 's linesColor has only one effect: change the color of "static lines" (identity, foldDistance, y=0 etc...).

Details

Based on my analysis of {tlf}, I've observed that the plotObject$plotConfiguration$lines setting, to where the ospsuite linesColor parameter go, is primarily used for two specific situations:

  1. Drawing diagonal (identity, foldlines), horizontal, and vertical lines (y=0 lines).
  2. Drawing the smoother lines. It's worth noting that the smoother lines exist in {tlf}, but they seem to not be used in ospsuite.

In the first case, these lines are not associated with any variables in the data. They are considered "static" and should not derive their colors from data groups. In the second case, the smoother lines are applied to each group in the data, and therefore, they should obtain colors from the respective groups (similar to how scatter points are handled).

To summarize, modifying the linesColor parameter in ospsuite will currently only impact the color of the "static" lines.

However, it's important to consider that in the future, the plotObject$plotConfiguration$lines setting might be utilized for other purposes. In such cases, should we create a separate setting specifically for what I refer to as "static lines."

What is your opinion on this @pchelle ?

pchelle commented 11 months ago

The list of {tlf} molecule plots could be split in 3 categories (except pie charts)

Because of the structure, it felt easier at the time to create a similar class that handled aesthetic properties and use the class to define how to plot points, lines, ribbons and errorbars separately.

pchelle commented 11 months ago

It may indeed be more transparent to expose the properties as actual named field (such as smoother and foldLines) so that users could more easily explore and tune the property.

This may require some fixes down the road because of the integration of {tlf} with {ospsuite} and {ospsuite.reportingengine} packages

@Yuri05 @PavelBal thoughts ?

pchelle commented 11 months ago

@Felixmil Unrelated question, is the x/y axis labeling of the example a default display from {tlf} ?

I just want to make sure this is not a bug, ticklabels are supposed to be handled by getLogTickLabels(ticks) and display e.g. $10^{-3}$, etc.

PavelBal commented 11 months ago

@pchelle From the user's perspective, I want to change the default color palette used to present the data sets. What I do (simplified) is:

DPC <- DefaultPlotConfiguration$new()

newColors <- c("red", "green", "blue")

DPC$linesColor = newColors
DPC$pointsColor = newColors

The side effect is that in the simulated-vs-observed, residuals-vs-time, and residuals-vs-simulated, the identity and fold-lines are not black any more, which is not the desired effect. Actually I do not see a use case where the user would like to change the color of those lines.

pchelle commented 11 months ago

@PavelBal In the simplified code, you actually set the colors of the lines to $\text{\color{red}{red}}$, $\text{\color{green}{green}}$ and $\text{\color{blue}{blue}}$ using DPC$linesColor = newColor or is this supposed to change the color of the errorbars ? In which case, the tlf::plotConfiguration field is errorbars (but I don't know how it is integrated to {ospsuite})

pchelle commented 11 months ago

Actually I do not see a use case where the user would like to change the color of those lines.

I agree from {ospsuite} perspective because the global theme and color palette is defined. However, from the {tlf} perspective, other themes and color palettes may be applied. In such case a tlf user should be free to display these lines with any color, linetype or size they wish.

PavelBal commented 11 months ago

In the simplified code, you actually set the colors of the lines to , and using DPC$linesColor = newColor or is this supposed to change the color of the errorbars ?

I simply want to change the overall appearance, so when I create an individual time profile (or a population, whatever), the default colors are the ones I provide. What do I have to set?

PavelBal commented 11 months ago

Like use something else instead of the default osp palette.

pchelle commented 11 months ago

Is it because ospsuite::DefaultPlotConfiguration sets the properties for all the plots including time profile and obs vs pred plots ?

Then, the best fix would be in my opinion, to prevent ospsuite::DefaultPlotConfiguration$linesColor to update the property tlf::ObsVsPredPlotConfiguration$lines$color and keep the property defined by the current tlf theme.

PavelBal commented 11 months ago

Then, the best fix would be in my opinion, to prevent ospsuite::DefaultPlotConfiguration$linesColor to update the property tlf::ObsVsPredPlotConfiguration$lines$color and keep the property defined by the current tlf theme.

Yes but again, then I cannot define have any other default colors for pred-vs-obs than the default ones?

pchelle commented 11 months ago

The new colors would be defined in both ospsuite::DefaultPlotConfiguration$pointsColor and ospsuite::DefaultPlotConfiguration$linesColor, wouldn't they ?

So, if tlf::ObsVsPredPlotConfiguration$points$color get the property from ospsuite::DefaultPlotConfiguration$pointsColor, the scatter plot will still follow the appropriate behavior while the identity and fold lines will just use the default that wrote you did not want to change.

PavelBal commented 11 months ago

Ok got it now. Thanks for clarification.

pchelle commented 11 months ago

Ok got it now. Thanks for clarification.

No problem :) Since the naming is similar I think it is really hard to split what property is {tlf}, what property is {ospsuite} and how they are currently integrated to each other.

Felixmil commented 11 months ago

Since ospsuite::DefaultPlotConfiguration$linesColor only impact static lines -which we don't want to change, why not forget about it and use only ospsuite::DefaultPlotConfiguration$pointsColor ? This will set a custom "group palette" without impacting static lines. Smooth, error bars etc will follow this palette just like the points since they are also mapped on the same groups.

example:

devtools::load_all()
#> ℹ Loading ospsuite
#> Loading required package: rClr
#> 
#> Loading the dynamic library for Microsoft .NET runtime...
#> Loaded Common Language Runtime version 4.0.30319.42000
#> Warning: package 'testthat' was built under R version 4.3.1

# load the simulation
sim <- loadTestSimulation("MinimalModel")
simResults <- importResultsFromCSV(
  simulation = sim,
  filePaths = getTestDataFilePath("Stevens_2012_placebo_indiv_results.csv")
)

# import observed data (will return a list of `DataSet` objects)
dataSet <- loadDataSetsFromExcel(
  xlsFilePath = getTestDataFilePath("CompiledDataSetStevens2012.xlsx"),
  importerConfiguration = loadDataImporterConfiguration(getTestDataFilePath("ImporterConfiguration.xml"))
)

# create a new instance and add datasets
myCombDat <- DataCombined$new()
myCombDat$addDataSets(dataSet)
myCombDat$addSimulationResults(
  simResults,
  quantitiesOrPaths = c(
    "Organism|Lumen|Stomach|Metformin|Gastric retention",
    "Organism|Lumen|Stomach|Metformin|Gastric retention distal",
    "Organism|Lumen|Stomach|Metformin|Gastric retention proximal"
  )
)

myCombDat$setGroups(
  names = c(
    "Organism|Lumen|Stomach|Metformin|Gastric retention",
    "Organism|Lumen|Stomach|Metformin|Gastric retention distal",
    "Organism|Lumen|Stomach|Metformin|Gastric retention proximal",
    "Stevens_2012_placebo.Placebo_total",
    "Stevens_2012_placebo.Placebo_distal",
    "Stevens_2012_placebo.Placebo_proximal"
  ),
  groups = c("Solid total", "Solid distal", "Solid proximal", "Solid total", "Solid distal", "Solid proximal")
)

# Plot Configuration
myPlotConfiguration <- DefaultPlotConfiguration$new()

# Change default color here
myPlotConfiguration$pointsColor <- viridis::viridis(3)

# Plot output
plotObservedVsSimulated(myCombDat, myPlotConfiguration, foldDistance = c(2,3,4))

Created on 2023-07-18 with reprex v2.0.2

PavelBal commented 11 months ago

Since ospsuite::DefaultPlotConfiguration$linesColor only impact static lines -which we don't want to change, why not forget about it and use only ospsuite::DefaultPlotConfiguration$pointsColor ? This will set a custom "group palette" without impacting static lines. Smooth, error bars etc will follow this palette just like the points since they are also mapped on the same groups.

lol, no, in this case the colors are changed in the obsverved-vs-simulated, but NOT in the individual time profile plots 🤯 This is so hard to comprehend...

Felixmil commented 11 months ago

Fix proposition in https://github.com/Open-Systems-Pharmacology/OSPSuite-R/pull/1271

Yuri05 commented 11 months ago

colors are evil (statement from a colorblind person 😆 )

Felixmil commented 11 months ago

@Yuri05 as deuteranopian, I can relate 😅