Fix ToDo 3

plot-config-portable/src/commonMain/kotlin/jetbrains/datalore/plot/config/DataMetaUtil.kt

@@ -21,6 +21,7 @@ object DataMetaUtil {
  require(!isDiscrete(variable)) { "toDiscrete() - variable already encoded: $variable" }
  return "$prefix$variable"
  }
+
  private fun fromDiscrete(variable: String): String {
  require(isDiscrete(variable)) { "fromDiscrete() - variable is not encoded: $variable" }
  return variable.removePrefix(prefix)
@@ -66,76 +67,76 @@ object DataMetaUtil {
  }
 
  /**
- * ToDo: update doc
- * returns new mappings to discrete variables and DataFrame with these discrete variables
+ * returns mappings and DataFrame extended with auto-generated discrete mappings and variables
  */
  fun createDataFrame(
  options: OptionsAccessor,
  commonData: DataFrame,
  commonDiscreteAes: Set<String>,
- commonMapping: Map<*, *>,
+ commonMappings: Map<*, *>,
  isClientSide: Boolean
  ): Pair<Map<*, *>, DataFrame> {
- // ToDo: rename to ownData
- val data = ConfigUtil.createDataFrame(options.get(Option.PlotBase.DATA))
- val combinedDfVars = DataFrameUtil.toMap(commonData) + DataFrameUtil.toMap(data)
+ val ownData = ConfigUtil.createDataFrame(options.get(Option.PlotBase.DATA))
+ val ownMappings = options.getMap(Option.PlotBase.MAPPING)
 
  if (isClientSide) {
  return Pair(
  // no new discrete mappings, all job was done on server side
- emptyMap<Any?, Any?>(), // ToDo: return ownMappings
+ ownMappings,
  // re-insert existing variables as discrete
- combinedDfVars // ToDo: you don't need 'combined'. You are re-inserting vars in ownData only.
+ DataFrameUtil.toMap(ownData)
  .filter { (varName, _) -> isDiscrete(varName) }
  .entries
- .fold(DataFrame.Builder(data)) { acc, (varName, values) ->
- val variable = findVariableOrFail(data, varName)
+ .fold(DataFrame.Builder(ownData)) { acc, (varName, values) ->
+ val variable = findVariableOrFail(ownData, varName)
  // re-insert as discrete
  acc.remove(variable)
  acc.putDiscrete(variable, values)
  }
  .build()
  )
 
- } else { // server side // ToDo: 'else' is not necessary
- val ownMappings = options.getMap(Option.PlotBase.MAPPING)
+ }
 
- // own names - not yet encoded, i.e. 'cyl'
- val ownDiscreteMappings = run {
- val ownDiscreteAes = getAsDiscreteAesSet(options.getMap(Option.Meta.DATA_META))
- ownMappings.filter { (aes, _) -> aes in ownDiscreteAes }
- }
+ // server side
 
- // common names - already encoded by PlotConfig, i.e. '@as_discrete@cyl'. Restore original name.
- val commonDiscreteVars = commonMapping.filterKeys { it in commonDiscreteAes }.variables().map(::fromDiscrete)
+ // own names not yet encoded, i.e. 'cyl'
+ val ownDiscreteMappings = run {
+ val ownDiscreteAes = getAsDiscreteAesSet(options.getMap(Option.Meta.DATA_META))
+ return@run ownMappings.filter { (aes, _) -> aes in ownDiscreteAes }
+ }
 
-  // Original (not encoded) discrete var names from both common and own mappings.
-  val combinedDiscreteVars = mutableSetOf<String>()
- combinedDiscreteVars += ownDiscreteMappings.variables()
- combinedDiscreteVars += commonDiscreteVars
+ // Original (not encoded) discrete var names from both common and own mappings.
+ val combinedDiscreteVars = run {
+ // common names already encoded by PlotConfig, i.e. '@as_discrete@cyl'. Restore original name.
+ val commonDiscreteVars = commonMappings.filterKeys { it in commonDiscreteAes }.variables().map(::fromDiscrete)
 
- // minus own non-discrete mappings (layer overrides plot)
  val ownSimpleVars = ownMappings.variables() - ownDiscreteMappings.variables()
- combinedDiscreteVars -= ownSimpleVars
-
- return Pair(
- ownMappings + ownDiscreteMappings.mapValues { (_, varName) ->
- require(varName is String)
- require(!isDiscrete(varName)) { "Already encoded discrete mapping: $varName" }
- toDiscrete(varName)
- },
- combinedDfVars
- .filter { (dfVarName, _) -> dfVarName in combinedDiscreteVars }
- .mapKeys { (dfVarName, _) -> createVariable(toDiscrete(dfVarName)) }
- .entries
- .fold(DataFrame.Builder(data)) { acc, (dfVar, values) -> acc.putDiscrete(dfVar, values)}
- .build()
- )
 
+ // minus own non-discrete mappings (simple layer var overrides discrete plot var)
+ return@run ownDiscreteMappings.variables() + commonDiscreteVars - ownSimpleVars
  }
+
+ val combinedDfVars = DataFrameUtil.toMap(commonData) + DataFrameUtil.toMap(ownData)
+
+ return Pair(
+ ownMappings + ownDiscreteMappings.mapValues { (_, varName) ->
+ require(varName is String)
+ require(!isDiscrete(varName)) { "Already encoded discrete mapping: $varName" }
+ toDiscrete(varName)
+ },
+ combinedDfVars
+ .filter { (dfVarName, _) -> dfVarName in combinedDiscreteVars }
+ .mapKeys { (dfVarName, _) -> createVariable(toDiscrete(dfVarName)) }
+ .entries
+ .fold(DataFrame.Builder(ownData)) { acc, (dfVar, values) -> acc.putDiscrete(dfVar, values) }
+ .build()
+ )
  }
+
 }
 
+
 private fun Map<*, *>.variables(): Set<String> {
  return values.map { it as String }.toSet()
 }

plot-config-portable/src/commonMain/kotlin/jetbrains/datalore/plot/config/LayerConfig.kt

@@ -29,7 +29,7 @@ import jetbrains.datalore.plot.config.Option.PlotBase.MAPPING
 class LayerConfig(
  layerOptions: Map<*, *>,
  sharedData: DataFrame,
- plotMapping: Map<*, *>,
+ plotMappings: Map<*, *>,
  plotDiscreteAes: Set<String>,
  val geomProto: GeomProto,
  statProto: StatProto,
@@ -70,24 +70,20 @@ class LayerConfig(
  }
 
  init {
- // ToDo: use either "xxxMapping" or "xxxMappings" (now: plotMapping but layerMappings)
  val (layerMappings, layerData) = createDataFrame(
  options = this,
  commonData = sharedData,
  commonDiscreteAes = plotDiscreteAes,
- commonMapping = plotMapping,
+ commonMappings = plotMappings,
  isClientSide = myClientSide
  )
 
  if (!myClientSide) {
- // ToDo: actually, checking
  update(MAPPING, layerMappings)
  }
 
  // mapping (inherit from plot) + 'layer' mapping
- // ToDo: replace: getMap(MAPPING) --> layerMappings
- // ToDo: rename to 'combinedMappings' for consistency.
- val mappingOptions = plotMapping + getMap(MAPPING)
+ val combinedMappings = plotMappings + layerMappings
 
  var combinedData: DataFrame
  if (!(sharedData.isEmpty || layerData.isEmpty) && sharedData.rowCount() == layerData.rowCount()) {
@@ -99,30 +95,30 @@ class LayerConfig(
  }
 
 
- var aesMapping: Map<Aes<*>, DataFrame.Variable>?
+ var aesMappings: Map<Aes<*>, DataFrame.Variable>?
  if (GeoPositionsDataUtil.hasGeoPositionsData(this) && myClientSide) {
  // join dataset and geo-positions data
  val dataAndMapping = GeoPositionsDataUtil.initDataAndMappingForGeoPositions(
  geomProto.geomKind,
  combinedData,
  GeoPositionsDataUtil.getGeoPositionsData(this),
- mappingOptions
+ combinedMappings
  )
  combinedData = dataAndMapping.first
- aesMapping = dataAndMapping.second
+ aesMappings = dataAndMapping.second
  } else {
- aesMapping = ConfigUtil.createAesMapping(combinedData, mappingOptions)
+ aesMappings = ConfigUtil.createAesMapping(combinedData, combinedMappings)
  }
 
  // auto-map variables if necessary
- if (aesMapping.isEmpty()) {
- aesMapping = DefaultAesAutoMapper.forGeom(geomProto.geomKind).createMapping(combinedData)
+ if (aesMappings.isEmpty()) {
+ aesMappings = DefaultAesAutoMapper.forGeom(geomProto.geomKind).createMapping(combinedData)
  if (!myClientSide) {
  // store used mapping options to pass to client.
  val autoMappingOptions = HashMap<String, Any>()
- for (aes in aesMapping.keys) {
+ for (aes in aesMappings.keys) {
  val option = Option.Mapping.toOption(aes)
- val variable = aesMapping[aes]!!.name
+ val variable = aesMappings[aes]!!.name
  autoMappingOptions[option] = variable
  }
  update(MAPPING, autoMappingOptions)
@@ -132,15 +128,14 @@ class LayerConfig(
  // exclude constant aes from mapping
  val constants = LayerConfigUtil.initConstants(this)
  if (constants.isNotEmpty()) {
- aesMapping = HashMap(aesMapping)
+ aesMappings = HashMap(aesMappings)
  for (aes in constants.keys) {
- aesMapping.remove(aes)
+ aesMappings.remove(aes)
  }
  }
 
- // ToDo: why we use "mappingOptions" here but "aesMapping" in getTooltipAesList(..)?
  // grouping
- explicitGroupingVarName = initGroupingVarName(combinedData, mappingOptions)
+ explicitGroupingVarName = initGroupingVarName(combinedData, combinedMappings)
 
  statKind = StatKind.safeValueOf(getString(STAT)!!)
  stat = statProto.createStat(statKind, mergedOptions)
@@ -156,11 +151,11 @@ class LayerConfig(
  }
 
  // tooltip aes list
- this.tooltipAes = getTooltipAesList(aesMapping)
+ this.tooltipAes = getTooltipAesList(aesMappings)
 
  val varBindings = LayerConfigUtil.createBindings(
  combinedData,
- aesMapping,
+ aesMappings,
  scaleProviderByAes,
  consumedAesSet
  )
@@ -225,26 +220,26 @@ class LayerConfig(
  return varBindings.find { it.aes == aes }?.scale
  }
 
- private fun getTooltipAesList(aesMapping: Map<Aes<*>, DataFrame.Variable>): List<Aes<*>>? {
+ private fun getTooltipAesList(mappings: Map<Aes<*>, DataFrame.Variable>): List<Aes<*>>? {
  // tooltip list is not defined - will be used default tooltips
- if (!has(TOOLTIP))
+ if (!has(TOOLTIP)) {
  return null
+ }
 
- val aesStringList = getStringList(TOOLTIP)
+ val tooltipAesSpec = getStringList(TOOLTIP)
 
  // check if all elements of list are aes
- (aesStringList - Aes.values().map { it.name }).firstOrNull {
- error("${it} is not aes name ")
- }
+ (tooltipAesSpec - Aes.values().map(Aes<*>::name)).firstOrNull { error("${it} is not aes name ") }
 
  // detach aes
- val aesList = Aes.values().filter { aesStringList.contains(it.name) }
+ val tooltipAesList = Aes.values().filter { it.name in tooltipAesSpec }
 
  // check if aes list matches to mapping
- if (!aesMapping.keys.containsAll(aesList))
- error("Aes list does not match to mapping")
+ if (!mappings.keys.containsAll(tooltipAesList)) {
+ error("Tooltip aes list does not match mappings: [${(tooltipAesList - mappings.keys).joinToString()}]")
+ }
 
- return aesList
+ return tooltipAesList
  }
 
  private companion object {

plot-config-portable/src/commonMain/kotlin/jetbrains/datalore/plot/config/PlotConfig.kt

@@ -47,15 +47,13 @@ abstract class PlotConfig(
  options = this,
  commonData = DataFrame.Builder.emptyFrame(),
  commonDiscreteAes = emptySet(),
- commonMapping = emptyMap<Any, Any>(),
+ commonMappings = emptyMap<Any, Any>(),
  isClientSide = isClientSide
  )
 
  sharedData = plotData
 
- // ToDo: 'update' on server side only.
- // ToDo: not very sure about checking 'isNotEmpty', but the behavion should be the same here and in LayerConfig.
- if (plotMappings.isNotEmpty()) {
+ if (!isClientSide) {
  update(MAPPING, plotMappings)
  }
 
@@ -125,7 +123,7 @@ abstract class PlotConfig(
  protected abstract fun createLayerConfig(
  layerOptions: Map<*, *>,
  sharedData: DataFrame?,
- plotMapping: Map<*, *>,
+ plotMappings: Map<*, *>,
  plotDiscreteAes: Set<String>,
  scaleProviderByAes: TypedScaleProviderMap
  ): LayerConfig

plot-config-portable/src/commonMain/kotlin/jetbrains/datalore/plot/config/PlotConfigClientSide.kt

@@ -48,7 +48,7 @@ class PlotConfigClientSide private constructor(opts: Map<String, Any>) : PlotCon
  override fun createLayerConfig(
  layerOptions: Map<*, *>,
  sharedData: DataFrame?,
- plotMapping: Map<*, *>,
+ plotMappings: Map<*, *>,
  plotDiscreteAes: Set<String>,
  scaleProviderByAes: TypedScaleProviderMap
  ): LayerConfig {
@@ -58,7 +58,7 @@ class PlotConfigClientSide private constructor(opts: Map<String, Any>) : PlotCon
  return LayerConfig(
  layerOptions,
  sharedData!!,
- plotMapping,
+ plotMappings,
  plotDiscreteAes,
  GeomProtoClientSide(geomKind),
  StatProto(),

plot-config-portable/src/commonMain/kotlin/jetbrains/datalore/plot/server/config/PlotConfigServerSide.kt

@@ -24,7 +24,7 @@ open class PlotConfigServerSide(opts: Map<String, Any>) : PlotConfig(opts) {
  override fun createLayerConfig(
  layerOptions: Map<*, *>,
  sharedData: DataFrame?,
- plotMapping: Map<*, *>,
+ plotMappings: Map<*, *>,
  plotDiscreteAes: Set<String>,
  scaleProviderByAes: TypedScaleProviderMap
  ): LayerConfig {
@@ -34,7 +34,7 @@ open class PlotConfigServerSide(opts: Map<String, Any>) : PlotConfig(opts) {
  return LayerConfig(
  layerOptions,
  sharedData!!,
- plotMapping,
+ plotMappings,
  plotDiscreteAes,
  GeomProto(geomKind),
  StatProto(),

plot-config/src/jvmTest/kotlin/plot/server/config/GGBunchTest.kt

@@ -61,7 +61,8 @@ class GGBunchTest {
  val plotSpec =
  mutableMapOf(
  Option.Meta.KIND to Option.Meta.Kind.PLOT,
- Option.Plot.LAYERS to listOf(geom)
+ Option.Meta.KIND to Option.Meta.Kind.PLOT,
+ Option.PlotBase.MAPPING to emptyMap<Any, Any>()
  )
 
  itemsList.add(