Set theme as feature list (#242)

* Set global theme as list of features.

* Fix init for global theme settings.

* Refactor global theme:

* Fix tests.

* Update future_changes.md: add link to the issue.

* Add merging to a one OptionsMap when specifying a list of features.

* Cleanup code.

future_changes.md

@@ -31,3 +31,5 @@
 ### Changed
 
 ### Fixed
+
+- `set_theme()` should accept "feature list" [[#657](https://github.com/JetBrains/lets-plot/issues/657)].

plot-api/src/commonMain/kotlin/org/jetbrains/letsPlot/LetsPlot.kt

@@ -5,15 +5,37 @@
 
 package org.jetbrains.letsPlot
 
+import org.jetbrains.letsPlot.core.spec.Option
 import org.jetbrains.letsPlot.frontend.NotebookFrontendContext
+import org.jetbrains.letsPlot.intern.Feature
+import org.jetbrains.letsPlot.intern.FeatureList
 import org.jetbrains.letsPlot.intern.OptionsMap
+import org.jetbrains.letsPlot.intern.ThemeOptionsUtil
 import org.jetbrains.letsPlot.intern.settings.GlobalSettings
 import org.jetbrains.letsPlot.intern.settings.createDefaultFrontendContext
 
 object LetsPlot {
     var frontendContext: FrontendContext = createDefaultFrontendContext()
 
-    var theme: OptionsMap? by GlobalSettings::theme
+    var theme: Feature? = null  // either null or OptionsMap
+        set(value) {
+            field = when (value) {
+                null -> null
+                is OptionsMap -> value
+                is FeatureList -> {
+                    val optionsMap = value.elements.map { it as? OptionsMap ?: error("theme: unsupported feature $it") }
+                    optionsMap.forEach {
+                        require(it.kind == Option.Plot.THEME) {
+                            "theme: wrong options type, expected `${Option.Plot.THEME}` but was `${it.kind}`"
+                        }
+                    }
+                    ThemeOptionsUtil.toSpec(optionsMap)?.let { mergedOptions ->
+                        OptionsMap(Option.Plot.THEME, mergedOptions)
+                    }
+                }
+                else -> throw IllegalArgumentException("Only `theme(...)`, `themeXxx()`, `flavorXxx()`, or a sum of them are supported")
+            }
+        }
 
     @Suppress("MemberVisibilityCanBePrivate")
     var apiVersion: String = "Unknown"

plot-api/src/commonMain/kotlin/org/jetbrains/letsPlot/gggrid.kt

@@ -8,10 +8,10 @@ package org.jetbrains.letsPlot
 import org.jetbrains.letsPlot.core.spec.Option.SubPlots
 import org.jetbrains.letsPlot.core.spec.Option.SubPlots.Grid.Scales.SHARE_ALL
 import org.jetbrains.letsPlot.core.spec.Option.SubPlots.Grid.Scales.SHARE_NONE
+import org.jetbrains.letsPlot.intern.OptionsMap
 import org.jetbrains.letsPlot.intern.figure.SubPlotsFigure
 import org.jetbrains.letsPlot.intern.figure.SubPlotsLayoutSpec
 import org.jetbrains.letsPlot.intern.filterNonNullValues
-import org.jetbrains.letsPlot.intern.settings.GlobalSettings
 
 /**
  *  Combines several plots on one figure, organized in a regular grid.
@@ -108,9 +108,11 @@ fun gggrid(
     @Suppress("NAME_SHADOWING")
     val plots = (plots.toList() + List(ncol - 1) { null }).take(len)
 
+    val features = listOfNotNull(LetsPlot.theme?.let { it as OptionsMap })
+
     return SubPlotsFigure(
         figures = plots,
         layout = layout,
-        features = listOfNotNull(GlobalSettings.theme)
+        features = features
     )
 }

plot-api/src/commonMain/kotlin/org/jetbrains/letsPlot/ggplot.kt

@@ -7,13 +7,12 @@ package org.jetbrains.letsPlot
 
 import org.jetbrains.letsPlot.intern.GenericAesMapping
 import org.jetbrains.letsPlot.intern.Plot
-import org.jetbrains.letsPlot.intern.settings.GlobalSettings
 
 fun letsPlot(data: Map<*, *>? = null, mapping: GenericAesMapping.() -> Unit = {}): Plot {
     return Plot(
         data = data,
         mapping = GenericAesMapping().apply(mapping).seal(),
-        features = listOfNotNull(GlobalSettings.theme)
+        features = listOfNotNull(LetsPlot.theme)
     )
 }
 

plot-api/src/commonMain/kotlin/org/jetbrains/letsPlot/intern/figure/SubPlotsFigure.kt

@@ -34,7 +34,7 @@ class SubPlotsFigure(
     fun toSpec(): MutableMap<String, Any> {
         val elementSpecs = figures.map { it?.toSpec() }
 
-        val globalThemeOptions = LetsPlot.theme?.options
+        val globalThemeOptions = LetsPlot.theme?.let { it as OptionsMap }?.options
         // Strip global theme options from plots in grid (see issue: LP-966).
         if (globalThemeOptions != null) {
             elementSpecs.filterNotNull().forEach { elementSpec ->

plot-api/src/commonMain/kotlin/org/jetbrains/letsPlot/intern/settings/GlobalSettings.kt

@@ -5,26 +5,11 @@
 
 package org.jetbrains.letsPlot.intern.settings
 
-import org.jetbrains.letsPlot.core.spec.Option.Plot.THEME
-import org.jetbrains.letsPlot.commons.intern.json.JsonSupport.parseJson
-import org.jetbrains.letsPlot.intern.OptionsMap
-import org.jetbrains.letsPlot.intern.filterNonNullValues
-
 
 // Environment variables
 const val ENV_HTML_ISOLATED_FRAME = "LETS_PLOT_HTML_ISOLATED_FRAME"
-const val ENV_PLOT_THEME = "LETS_PLOT_PLOT_THEME"
 
 
 internal object GlobalSettings {
     val isolatedFrameContext: Boolean get() = Env.getBool(ENV_HTML_ISOLATED_FRAME) ?: false
-
-    var theme: OptionsMap? = Env.get(ENV_PLOT_THEME)?.let { OptionsMap(THEME, parseJson(it).filterNonNullValues()) }
-        set(value) {
-            require(value == null || value.kind == THEME) {
-                "Wrong options type. Expected `$THEME` but was `${value!!.kind}`"
-            }
-
-            field = value
-        }
 }

plot-api/src/jvmTest/kotlin/org/jetbrains/letsPlot/ThemePlotGridTest.kt

@@ -11,6 +11,7 @@ import org.jetbrains.letsPlot.core.spec.Option
 import org.jetbrains.letsPlot.intern.SubPlotsAssert.Companion.assertThat
 import org.jetbrains.letsPlot.intern.figure.SubPlotsFigure
 import org.jetbrains.letsPlot.themes.flavorDarcula
+import org.jetbrains.letsPlot.themes.theme
 import org.jetbrains.letsPlot.themes.themeBW
 import org.jetbrains.letsPlot.themes.themeGrey
 import org.jetbrains.letsPlot.themes.themeLight
@@ -136,6 +137,32 @@ class ThemePlotGridTest {
         }
     }
 
+    @Test
+    fun `grid global theme - override named theme but keep global flavor`() {
+        // Set global setting!
+        LetsPlot.theme = themeGrey() + flavorDarcula()
+
+        try {
+            val p = gggrid(listOf(ggplot())) + themeLight() // Override global theme
+            assertThemeSpec(
+                p,
+                featureCount = 2,
+                expected = mapOf(
+                    "name" to "light",
+                    "flavor" to "darcula",
+                )
+            )
+
+            // Global theme should be stripped from the figure spec in grid
+            val fig = (p.toSpec()["figures"] as List<Any?>)[0] as Map<*, *>
+            assertEquals("plot", fig["kind"]) // Make sure it's a plot
+            assertFalse(fig.containsKey("theme"))
+        } finally {
+            // Clear global setting
+            LetsPlot.theme = null
+        }
+    }
+
     @Test
     fun `grid global theme override cancelled`() {
         // Set global setting!
@@ -170,6 +197,98 @@ class ThemePlotGridTest {
         }
     }
 
+    // with theme as list of features
+
+    @Test
+    fun `grid global theme as feature list applied`() {
+        // Set global settings as combination of theme settings
+        LetsPlot.theme = themeGrey() + flavorDarcula() + theme().legendPositionBottom()
+
+        try {
+            val p = gggrid(listOf(ggplot()))
+            assertThemeSpec(
+                p,
+                featureCount = 1,
+                expected = mapOf(
+                    "name" to "grey",
+                    "flavor" to "darcula",
+                    "legend_position" to "bottom"
+                )
+            )
+
+            // Global theme should be stripped from the figure spec in grid
+            val fig = (p.toSpec()["figures"] as List<Any?>)[0] as Map<*, *>
+            assertEquals("plot", fig["kind"]) // Make sure it's a plot
+            assertFalse(fig.containsKey("theme"))
+        } finally {
+            // Clear global setting
+            LetsPlot.theme = null
+        }
+    }
+
+    @Test
+    fun `grid global theme as feature list override`() {
+        // Set global settings as combination os themes
+        LetsPlot.theme = themeGrey() + flavorDarcula() // + theme().legendPositionBottom()
+
+        try {
+            val p = gggrid(listOf(ggplot())) + themeLight() // Override global named theme
+            assertThemeSpec(
+                p,
+                featureCount = 2,
+                expected = mapOf(
+                    "name" to "light",
+                    "flavor" to "darcula",              // Keep flavor
+                    // "legend_position" to "bottom"       // and theme option
+                )
+            )
+
+            // Global theme should be stripped from the figure spec in grid
+            val fig = (p.toSpec()["figures"] as List<Any?>)[0] as Map<*, *>
+            assertEquals("plot", fig["kind"]) // Make sure it's a plot
+            assertFalse(fig.containsKey("theme"))
+        } finally {
+            // Clear global setting
+            LetsPlot.theme = null
+        }
+    }
+
+    @Test
+    fun `grid global theme as feature list override cancelled`() {
+        // Set global settings as combination os themes
+        LetsPlot.theme = themeGrey() + flavorDarcula() // + theme().legendPositionBottom()
+
+        try {
+            val element = ggplot() + themeBW()  // this should cancel global theme for this figure
+            val p = gggrid(listOf(element)) + themeLight()
+            assertThemeSpec(
+                p,
+                featureCount = 2,
+                expected = mapOf(
+                    "name" to "light",
+                    "flavor" to "darcula",              // Keep flavor
+                )
+            )
+
+            // Global theme should be stripped from the figure spec in grid
+            val fig = (p.toSpec()["figures"] as List<Any?>)[0] as Map<*, *>
+            assertEquals("plot", fig["kind"]) // Make sure it's a plot
+            assertTrue(fig.containsKey("theme"))
+
+            assertEquals(
+                mapOf(
+                    "name" to "bw",
+                    "flavor" to "darcula",              // Add flavor
+                 // "legend_position" to "bottom"       // and theme option
+                ),
+                fig["theme"]
+            )
+        } finally {
+            // Clear global setting
+            LetsPlot.theme = null
+        }
+    }
+
     private companion object {
         fun assertThemeSpec(
             figure: SubPlotsFigure,

plot-api/src/jvmTest/kotlin/org/jetbrains/letsPlot/ThemeTest.kt

@@ -213,4 +213,23 @@ class ThemeTest {
 
         LetsPlot.theme = null
     }
+
+    @Test
+    fun `global theme as feature list`() {
+        LetsPlot.theme = themeNone() + flavorDarcula() + theme().legendPositionBottom()
+        try {
+            val p = ggplot()
+            assertEquals(
+                mapOf(
+                    "name" to "none",
+                    "flavor" to "darcula",
+                    "legend_position" to "bottom"
+                ),
+                p.toSpec()[Option.Plot.THEME]
+            )
+        } finally {
+            // Clear global setting
+            LetsPlot.theme = null
+        }
+    }
 }