Code-review

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

@@ -20,12 +20,14 @@ object DataMetaUtil {
  if (variable.startsWith(prefix)) {
  variable.removePrefix(prefix)
  } else {
+ // ToDo: why return null? Lets throw IllegalArgumentException
  null
  }
 
  /**
  @returns Map<aes, annotation>
  */
+ // ToDo: rename to getAesMappingAnnotations
  private fun getMappingAnnotation(options: Map<*, *>): Map<String, String> {
  return options
  .getMaps(MappingAnnotation.TAG)
@@ -36,10 +38,12 @@ object DataMetaUtil {
  /**
  @returns discrete aes
  */
+ // ToDo: rename to getAsDiscreteAesSet
  fun getDiscreteAes(options: Map<*, *>): Set<String> {
  return getMappingAnnotation(options).filterValues(DISCRETE::equals).keys
  }
 
+ // ToDo: rename to createScaleSpecs
  fun processDiscreteScales(plotOptions: Map<String, Any>): List<MutableMap<String, Any>> {
  val plotDiscreteAes = plotOptions
  .getMap(Option.Meta.DATA_META)
@@ -65,6 +69,7 @@ object DataMetaUtil {
  /**
  * returns new mappings to discrete variables and DataFrame with these discrete variables
  */
+ // ToDo: rename to createLayerDataFrame
  fun processDiscreteData(
  options: OptionsAccessor,
  commonData: DataFrame,
@@ -86,7 +91,9 @@ object DataMetaUtil {
  val insert: (b: DataFrame.Builder, name: String, values: List<*>) -> DataFrame.Builder =
  if (isClientSide) {
  { b, name, values ->
+ // ToDo: Why 'name' have to be present in data?
  val variable = findVariableOrFail(data, name)
+ // re-insert as discrete
  b.remove(variable)
  b.putDiscrete(variable, values)
  }
@@ -104,14 +111,14 @@ object DataMetaUtil {
  // Special case: ggplot(aes(color=as_discrete('cyl'))) + geom_point(data=mpg)
  // Server side:
  // - plot: ('color' to 'cyl') -> ('color' to '@as_discrete@cyl')
- // - layer: ('color' to '@as_discrete@cyl') -> ('color' to 'cyl')
+ // - layer: ('color' to '@as_discrete@cyl') -> ('color' to 'cyl') // ToDo: ???
  // Client side:
  // - plot : ('color' to '@as_discrete@cyl') -> ('color' to '@as_discrete@cyl')
  // - layer: ('color' to '@as_discrete@cyl') -> ('color' to '@as_discrete@cyl')
  val commonDiscreteMappings = commonMapping
  .map { (aes, varName) -> aes as String to varName as String }.toMap()
  .filterKeys { it in commonDiscreteAes } // common discrete mapping
- .mapValues { (_, varName) -> decode(varName) }
+ .mapValues { (_, varName) -> decode(varName) } // ToDo: is this server-specific?
 
  val combinedDiscreteMappingVars = (commonDiscreteMappings + ownDiscreteMappings).values
 

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

@@ -79,6 +79,8 @@ class LayerConfig(
  )
 
  if (discreteMappings.isNotEmpty()) {
+ // ToDo: layer own mappings should override?
+ // ToDo: only necessary on server side?
  update(MAPPING, getMap(MAPPING) + discreteMappings)
  }
 

python-package/lets_plot/mapping.py

@@ -2,6 +2,8 @@
 # Use of this source code is governed by the MIT license that can be found in the LICENSE file.
 
 
+# ToDo: remane to MappingMeta
+# ToDo: fields: `variable`, `annotation`
 class VariableMeta:
  def __init__(self, name, kind):
  if name is None:
@@ -44,6 +46,7 @@ def as_discrete(var_name):
  >>> ggplot(df, aes(x='x', y='y')) + geom_point(aes(color=pm.as_discrete('a')), size=9)
  """
  if isinstance(var_name, str):
+ # ToDo: annotate with the function name: `as_discrete`
  return VariableMeta(var_name, 'discrete')
  # aes(x=factor([1, 2, 3])) - pass as is
  return var_name

python-package/lets_plot/plot/util.py

@@ -38,6 +38,7 @@ def as_annotated_data(raw_data: Any, raw_mapping: dict) -> Tuple:
  mapping_meta = []
 
  if raw_mapping is not None:
+ # ToDo: rename ket --> aes
  for key, variable in raw_mapping.as_dict().items():
  if key == 'name': # ignore FeatureSpec.name property
  continue