Autorotate discrete (#1032)

* Refactoring of choosing orientation. Move logic into init method of LayerConfig. * Refactor isAesDiscrete. Add unit-tests. * Disable automatic orientation selection if it is specified by the user. * Move initialization of isYOrientation variable.
JetBrains · Apr 8, 2024 · 85dbbed · 85dbbed
1 parent 161dcdc
commit 85dbbed
Show file tree

Hide file tree

Showing 8 changed files with 5,507 additions and 11 deletions.
diff --git a/docs/dev/notebooks/auto_orientation_discretes.ipynb b/docs/dev/notebooks/auto_orientation_discretes.ipynb
diff --git a/docs/dev/notebooks/auto_orientation_discretes_with_user_input.ipynb b/docs/dev/notebooks/auto_orientation_discretes_with_user_input.ipynb
diff --git a/docs/f-24b/auto_rotate.ipynb b/docs/f-24b/auto_rotate.ipynb
diff --git a/future_changes.md b/future_changes.md
@@ -22,6 +22,11 @@
 
  See: [example notebook](https://nbviewer.org/github/JetBrains/lets-plot/blob/master/docs/f-24b/param_size_unit.ipynb).
 
+- Automatically choose `orientation="y"` when aes y is discrete [[#558](https://github.com/JetBrains/lets-plot/issues/558)].
+
+ See: [example notebook](https://nbviewer.org/github/JetBrains/lets-plot/blob/master/docs/f-24b/auto_rotate.ipynb).
+
+
 ### Changed
   
 ### Fixed

diff --git a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/data/DataFrameUtil.kt b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/data/DataFrameUtil.kt
@@ -55,6 +55,11 @@ object DataFrameUtil {
  )
  }
 
+ fun findVariableOrNull(data: DataFrame, varName: String): DataFrame.Variable? {
+ if (!hasVariable(data, varName)) return null
+ return findVariableOrFail(data, varName)
+ }
+
  fun isNumeric(data: DataFrame, varName: String): Boolean {
  return data.isNumeric(findVariableOrFail(data, varName))
  }

diff --git a/plot-stem/src/commonMain/kotlin/org/jetbrains/letsPlot/core/spec/config/DataConfigUtil.kt b/plot-stem/src/commonMain/kotlin/org/jetbrains/letsPlot/core/spec/config/DataConfigUtil.kt
@@ -135,6 +135,34 @@ internal object DataConfigUtil {
  )
  }
 
+ fun isAesDiscrete(
+ aes: Aes<*>,
+ sharedData: DataFrame,
+ layerData: DataFrame,
+ sharedMappings: Map<String, String>,
+ layerMappings: Map<String, String>,
+ combinedDiscreteMappings: Map<String, String>
+ ): Boolean {
+ // Check if the aes is marked with as_discrete()
+ if (combinedDiscreteMappings.containsKey(aes.name)) return true
+
+ // Check if the aes is discrete.
+ val varName = layerMappings[aes.name] ?: sharedMappings[aes.name] ?: return false
+ // The DataFrame selection logic is identical to that of the layerMappingsAndCombinedData() function.
+ val layerVar = DataFrameUtil.findVariableOrNull(layerData, varName)
+ val sharedVar = DataFrameUtil.findVariableOrNull(sharedData, varName)
+
+ if (layerVar != null) {
+ return layerData.isDiscrete(layerVar)
+ }
+
+ if (sharedVar != null) {
+ return sharedData.isDiscrete(sharedVar)
+ }
+
+ return false
+ }
+
  fun combinedDataWithDataMeta(
  rawCombinedData: DataFrame,
  varBindings: List<VarBinding>,

diff --git a/plot-stem/src/commonMain/kotlin/org/jetbrains/letsPlot/core/spec/config/LayerConfig.kt b/plot-stem/src/commonMain/kotlin/org/jetbrains/letsPlot/core/spec/config/LayerConfig.kt
@@ -90,17 +90,6 @@ class LayerConfig(
  }
 
  val isYOrientation: Boolean
- get() = when (hasOwn(ORIENTATION)) {
- true -> getString(ORIENTATION)?.lowercase()?.let {
- when (it) {
- "y" -> true
- "x" -> false
- else -> throw IllegalArgumentException("$ORIENTATION expected x|y but was $it")
- }
- } ?: false
-
- false -> false
- }
 
  // Marginal layers
  val isMarginal: Boolean = getBoolean(MARGINAL, false)
@@ -157,6 +146,42 @@ class LayerConfig(
  ownDiscreteAes = DataMetaUtil.getAsDiscreteAesSet(getMap(DATA_META))
  )
 
+ isYOrientation = when (hasOwn(ORIENTATION)) {
+ true -> getString(ORIENTATION)?.lowercase()?.let {
+ when (it) {
+ "y" -> true
+ "x" -> false
+ else -> throw IllegalArgumentException("$ORIENTATION expected x|y but was $it")
+ }
+ } ?: false
+
+ false ->
+ if (!clientSide
+ && isOrientationApplicable()
+ && !DataConfigUtil.isAesDiscrete(
+ Aes.X,
+ plotData,
+ ownData,
+ plotMappings,
+ layerMappings,
+ combinedDiscreteMappings
+ )
+ && DataConfigUtil.isAesDiscrete(
+ Aes.Y,
+ plotData,
+ ownData,
+ plotMappings,
+ layerMappings,
+ combinedDiscreteMappings
+ )
+ ) {
+ setOrientationY()
+ true
+ } else {
+ false
+ }
+ }
+
  val consumedAesSet: Set<Aes<*>> = renderedAes.toSet().let {
  when (clientSide) {
  true -> it
@@ -279,6 +304,31 @@ class LayerConfig(
  combinedDataValid = false
  }
 
+ private fun isOrientationApplicable(): Boolean {
+ val isSuitableGeomKind = geomProto.geomKind in listOf(
+ GeomKind.BAR,
+ GeomKind.BOX_PLOT,
+ GeomKind.VIOLIN,
+ GeomKind.LOLLIPOP,
+ GeomKind.Y_DOT_PLOT
+ )
+ val isSuitableStatKind = statKind in listOf(
+ StatKind.COUNT,
+ StatKind.SUMMARY,
+ StatKind.BOXPLOT,
+ StatKind.BOXPLOT_OUTLIER,
+ StatKind.YDOTPLOT,
+ StatKind.YDENSITY
+ )
+
+ return isSuitableGeomKind || isSuitableStatKind
+ }
+
+ private fun setOrientationY() {
+ check(!clientSide)
+ update(ORIENTATION, "y")
+ }
+
  fun hasExplicitGrouping(): Boolean {
  return explicitGroupingVarName != null
  }

diff --git a/plot-stem/src/jvmTest/kotlin/org/jetbrains/letsPlot/core/spec/config/IsAesDiscreteTest.kt b/plot-stem/src/jvmTest/kotlin/org/jetbrains/letsPlot/core/spec/config/IsAesDiscreteTest.kt
@@ -0,0 +1,216 @@
+/*
+ * Copyright (c) 2024. JetBrains s.r.o.
+ * Use of this source code is governed by the MIT license that can be found in the LICENSE file.
+ */
+
+package org.jetbrains.letsPlot.core.spec.config
+
+import org.jetbrains.letsPlot.core.plot.base.Aes
+import org.jetbrains.letsPlot.core.plot.base.data.DataFrameUtil
+import kotlin.test.*
+
+class IsAesDiscreteTest {
+ private val data = mapOf(
+ "code" to listOf("a", "b", "c"),
+ "code_num" to listOf(4.0, 5.0, 6.0),
+ "value" to listOf(1.0, -5.0, 6.0)
+ )
+
+ private val dataInv = mapOf(
+ "code" to listOf(2.0, 3.0, 7.0),
+ "value" to listOf("f", "g", "h")
+ )
+
+ @Test
+ fun `data and mapping in plot`() {
+ val plotData = DataFrameUtil.fromMap(data)
+ val layerData = DataFrameUtil.fromMap(emptyMap<String, String>())
+ val plotMapping = mapOf(
+ "x" to "code",
+ "y" to "value"
+ )
+ val layerMapping = emptyMap<String, String>()
+ val asDiscreteMapping = emptyMap<String, String>()
+
+ val isXDiscrete = DataConfigUtil.isAesDiscrete(
+ Aes.X,
+ plotData,
+ layerData,
+ plotMapping,
+ layerMapping,
+ asDiscreteMapping
+ )
+ val isYDiscrete = DataConfigUtil.isAesDiscrete(
+ Aes.Y,
+ plotData,
+ layerData,
+ plotMapping,
+ layerMapping,
+ asDiscreteMapping
+ )
+
+ assertTrue(isXDiscrete)
+ assertFalse(isYDiscrete)
+ }
+
+ @Test
+ fun `data and mapping in layer`(){
+ val plotData = DataFrameUtil.fromMap(emptyMap<String, String>())
+ val layerData = DataFrameUtil.fromMap(data)
+ val plotMapping = emptyMap<String, String>()
+ val layerMapping = mapOf(
+ "x" to "code",
+ "y" to "value"
+ )
+ val asDiscreteMapping = emptyMap<String, String>()
+
+ val isXDiscrete = DataConfigUtil.isAesDiscrete(
+ Aes.X,
+ plotData,
+ layerData,
+ plotMapping,
+ layerMapping,
+ asDiscreteMapping
+ )
+ val isYDiscrete = DataConfigUtil.isAesDiscrete(
+ Aes.Y,
+ plotData,
+ layerData,
+ plotMapping,
+ layerMapping,
+ asDiscreteMapping
+ )
+
+ assertTrue(isXDiscrete)
+ assertFalse(isYDiscrete)
+ }
+
+ @Test
+ fun `data in plot but mapping in layer`(){
+ val plotData = DataFrameUtil.fromMap(data)
+ val layerData = DataFrameUtil.fromMap(emptyMap<String, String>())
+ val plotMapping = emptyMap<String, String>()
+ val layerMapping = mapOf(
+ "x" to "code",
+ "y" to "value"
+ )
+ val asDiscreteMapping = emptyMap<String, String>()
+
+ val isXDiscrete = DataConfigUtil.isAesDiscrete(
+ Aes.X,
+ plotData,
+ layerData,
+ plotMapping,
+ layerMapping,
+ asDiscreteMapping
+ )
+ val isYDiscrete = DataConfigUtil.isAesDiscrete(
+ Aes.Y,
+ plotData,
+ layerData,
+ plotMapping,
+ layerMapping,
+ asDiscreteMapping
+ )
+
+ assertTrue(isXDiscrete)
+ assertFalse(isYDiscrete)
+ }
+
+
+ @Test
+ fun `aes use as_discrete()`() {
+ val plotData = DataFrameUtil.fromMap(data)
+ val layerData = DataFrameUtil.fromMap(emptyMap<String, String>())
+ val plotMapping = mapOf(
+ "x" to "code",
+ "y" to "value"
+ )
+ val layerMapping = emptyMap<String, String>()
+ val asDiscreteMapping = mapOf(
+ "x" to "code",
+ "y" to "value"
+ )
+
+ val isXDiscrete = DataConfigUtil.isAesDiscrete(
+ Aes.X,
+ plotData,
+ layerData,
+ plotMapping,
+ layerMapping,
+ asDiscreteMapping
+ )
+ val isYDiscrete = DataConfigUtil.isAesDiscrete(
+ Aes.Y,
+ plotData,
+ layerData,
+ plotMapping,
+ layerMapping,
+ asDiscreteMapping
+ )
+
+ assertTrue(isXDiscrete)
+ assertTrue(isYDiscrete)
+ }
+
+ @Test
+ fun `data and mapping in plot with layer`() {
+ val plotData = DataFrameUtil.fromMap(data)
+ val layerData = DataFrameUtil.fromMap(emptyMap<String, String>())
+ val plotMapping = mapOf(
+ "x" to "code",
+ "y" to "value"
+ )
+ val layerMapping = mapOf(
+ "x" to "code_num"
+ )
+ val asDiscreteMapping = emptyMap<String, String>()
+
+ val isXDiscrete = DataConfigUtil.isAesDiscrete(
+ Aes.X,
+ plotData,
+ layerData,
+ plotMapping,
+ layerMapping,
+ asDiscreteMapping
+ )
+ assertFalse(isXDiscrete)
+ }
+
+ @Test
+ fun `different data in plot and layer`() {
+ val plotData = DataFrameUtil.fromMap(data)
+ val layerData = DataFrameUtil.fromMap(dataInv)
+ val plotMapping = mapOf(
+ "x" to "code",
+ "y" to "value"
+ )
+ val layerMapping = mapOf(
+ "x" to "code",
+ "y" to "value"
+ )
+ val asDiscreteMapping = emptyMap<String, String>()
+
+ val isXDiscrete = DataConfigUtil.isAesDiscrete(
+ Aes.X,
+ plotData,
+ layerData,
+ plotMapping,
+ layerMapping,
+ asDiscreteMapping
+ )
+ val isYDiscrete = DataConfigUtil.isAesDiscrete(
+ Aes.Y,
+ plotData,
+ layerData,
+ plotMapping,
+ layerMapping,
+ asDiscreteMapping
+ )
+
+ assertFalse(isXDiscrete)
+ assertTrue(isYDiscrete)
+ }
+
+
+}