Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Long legend #504

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from
Draft

Long legend #504

wants to merge 14 commits into from

Conversation

PavelBal
Copy link
Member

Fixes #503

@PavelBal PavelBal marked this pull request as draft July 29, 2023 20:49
@PavelBal PavelBal requested a review from Felixmil July 29, 2023 20:49
@PavelBal
Copy link
Member Author

@Felixmil This PR introduces automatic adjustment of text sizes of a plot that is exported through ExportConfiguration.

Some test code:

###### Test for long captions
myDc <- DataCombined$new()
ds <- DataSet$new("testDs")
ds$setValues(1, 1)
myDc$addDataSets(ds)

myDc2 <- DataCombined$new()
ds2 <- DataSet$new("testssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssDs!")
ds2$setValues(1, 1)
myDc2$addDataSets(list(ds, ds2))

plotConfig <- createEsqlabsPlotConfiguration()
plotConfig$title <- "Even loooooooooooooo00000000000000000000000000000000000000ooooooooooooooonger titleeeeeeee!"
singlePlot <- plotIndividualTimeProfile(myDc, plotConfig)
plotConfig$title <- "plot b"
singlePlot2 <- plotIndividualTimeProfile(myDc2, plotConfig)

plotGridConfiguration <- createEsqlabsPlotGridConfiguration()
plotGridConfiguration$title <- "lssooooooooooooooooooooooo000000000000000000000000000000000000000000000oooon"
plotGridConfiguration$addPlots(list(singlePlot, singlePlot2))

plotCombined <- plotGrid(plotGridConfiguration)

exportConfiguration <- createEsqlabsExportConfiguration(projectConfiguration = projectConfiguration)
exportConfiguration$name <- "testExport"
exportConfiguration$savePlot(plotCombined, autoscaleText = TRUE)
exportConfiguration$savePlot(singlePlot2, autoscaleText = TRUE)

Example of a plot with autoscaleText = FALSE:

testExport_

The same plot exported with autoscaleText = TRUE:
testExport

Legend entries are still causing lot of trouble, as:

  • Symbol size in the legend is too big (much bigger than in the actual plot)
  • Symbol size is not scaling with the text. Would it be scaled, the overall shrinking required to fit the text would be smaller and the text could be bigger (and it would be more consistent)
  • Spacing between legend entries and between the symbol and the text is too big.

@Felixmil I twisted my brain trying to figure out how to change the size of legend symbols, but I could not figure it out. Maybe you will have better luck. Please try to find ways to change the above-mentioned properties. I could take care of the (initial) fine-tuning.

@Felixmil
Copy link
Collaborator

Felixmil commented Jul 31, 2023

@PavelBal If I understood correctly, only the exported version of the plots will have adjusted text/keys sizes, not the plot Object.

Shouldn't we perform these modifications directly in the plot generation processes (plot and grids) so that objects and exports are identical ? If yes, I can move reuse what you implemented here in {tlf} (for plots) and {ospsuite} (for plotgrids).

@PavelBal
Copy link
Member Author

If this can be done at earlier steps than export - even better, go ahead. I wasn't sure because the size of the plot in cm is defined in the export configuration, I am not that familiar with gglop to judge if the scaling can be done earlier.

@codecov
Copy link

codecov bot commented Aug 1, 2023

Codecov Report

Merging #504 (9161f57) into main (0c61e67) will decrease coverage by 0.51%.
The diff coverage is 68.11%.

❗ Current head 9161f57 differs from pull request most recent head f37ade5. Consider uploading reports for the commit f37ade5 to get more accurate results

@@            Coverage Diff             @@
##             main     #504      +/-   ##
==========================================
- Coverage   82.06%   81.56%   -0.51%     
==========================================
  Files          30       30              
  Lines        1812     1871      +59     
==========================================
+ Hits         1487     1526      +39     
- Misses        325      345      +20     
Files Changed Coverage Δ
R/esqlabs-plot-export-configuration.R 69.11% <64.40%> (-30.89%) ⬇️
R/utilities-figures.R 92.68% <90.00%> (-0.24%) ⬇️

... and 2 files with indirect coverage changes

… ARE NOT SCALED YET!)

- Collapsing title strings (WHY?)
@Felixmil
Copy link
Collaborator

Felixmil commented Aug 7, 2023

Here is what can be obtained with the new TLF branch:

knitr::opts_chunk$set(fig.width = 7, fig.asp = 1,  dpi = 300, warning = FALSE)
library(esqlabsR)
#> Loading required package: ospsuite
#> Loading required package: rClr
#> Loading the dynamic library for Microsoft .NET runtime...
#> Loaded Common Language Runtime version 4.0.30319.42000
###### Test for long captions
myDc <- DataCombined$new()
ds <- DataSet$new("LegendsCanHaveVeryLongStrings|BecauseTLFNowWrapsThemOn|SpacesorSpecialCharacters")
ds$setValues(1, 1)
myDc$addDataSets(ds)

myDc2 <- DataCombined$new()
ds2 <- DataSet$new("LegendsCanHaveVeryLongStringsBecause|TLFNowWrapsThemOnSpacesorSpecialCharacters")
ds2$setValues(1, 1)
myDc2$addDataSets(list(ds, ds2))

plotConfig <- createEsqlabsPlotConfiguration()
plotConfig$titleSize <- 9
plotConfig$title <- "ThisPlotTitleIsVeryLong ButTLFCanNowWrapLongStringsOnSpaces WhichShouldBeCommonOnPlotTitles!"
singlePlot <- plotIndividualTimeProfile(myDc, plotConfig)
plotConfig$title <-  "ThisPlotTitleIsAlsoVeryLong ButTLFCanNowWrapLongStringsOnSpaces WhichShouldBeCommonOnPlotTitles!"
singlePlot2 <- plotIndividualTimeProfile(myDc2, plotConfig)

plotGridConfiguration <- createEsqlabsPlotGridConfiguration()
plotGridConfiguration$title <- "ThisPlotGridHasAVeryLongTitle ThatWillFitOnTopOfThePlots BecauseTLFCanNowWrapTextOnSpaces"
plotGridConfiguration$addPlots(list(singlePlot, singlePlot2))

plotGrid(plotGridConfiguration)

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

@PavelBal
Copy link
Member Author

PavelBal commented Aug 7, 2023

This will definitely reduce the number of scaling required. For use cases as in the example above, I think we should still keep this feature. Example:

Unscaled:
testExport_unscaled

Scaled:
testExport

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fit text into plot size when exporting
2 participants