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

factor -> as_discrete #117

Merged
merged 12 commits into from
Apr 17, 2020
Prev Previous commit
Next Next commit
Fix ToDo 2
  • Loading branch information
IKupriyanov-HORIS committed Apr 17, 2020
commit db981e496a4eb6ea9fa8fb01188c05f0c2e4de2a
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,23 @@
package jetbrains.datalore.plot.config

import jetbrains.datalore.plot.base.DataFrame
import jetbrains.datalore.plot.base.data.DataFrameUtil
import jetbrains.datalore.plot.base.data.DataFrameUtil.createVariable
import jetbrains.datalore.plot.base.data.DataFrameUtil.findVariableOrFail
import jetbrains.datalore.plot.base.data.DataFrameUtil.variables
import jetbrains.datalore.plot.config.Option.Meta.MappingAnnotation
import jetbrains.datalore.plot.config.Option.Meta.MappingAnnotation.DISCRETE

object DataMetaUtil {
private const val prefix = "@as_discrete@"

// ToDo: let's add an assert 'not isDiscrete(variable)'
private fun isDiscrete(variable: String) = variable.startsWith(prefix)
public fun toDiscrete(variable: String) = "$prefix$variable"

public fun toDiscrete(variable: String): String {
require(!isDiscrete(variable)) { "toDiscrete() - variable already encoded: $variable" }
return "$prefix$variable"
}
private fun fromDiscrete(variable: String): String {
require(isDiscrete(variable)) { "fromDiscrete() - ${variable} is not discrete" }
require(isDiscrete(variable)) { "fromDiscrete() - variable is not encoded: $variable" }
return variable.removePrefix(prefix)
}

Expand Down Expand Up @@ -73,32 +76,15 @@ object DataMetaUtil {
isClientSide: Boolean
): Pair<Map<*, *>, DataFrame> {
val data = ConfigUtil.createDataFrame(options.get(Option.PlotBase.DATA))
val ownMappings = options.getMap(Option.PlotBase.MAPPING)
val ownDiscreteMappings = run {
val ownDiscreteAes = getAsDiscreteAesSet(options.getMap(Option.Meta.DATA_META))
ownMappings.filter { (aes, _) -> aes in ownDiscreteAes }
}

// ToDo: ownDiscreteMappings/commonDiscreteMappings - it seems you doesn't need maps, only sets of varnames.
val commonDiscreteMappings = commonMapping.filterKeys { it in commonDiscreteAes }
val combinedDfVars = DataFrameUtil.toMap(commonData) + DataFrameUtil.toMap(data)

val combinedDfVars = run {
// ToDo: use DataFrameUtil.toMap
val ownDfVars = variables(data).mapValues { (_, dfVar) -> data[dfVar] }
val commonDfVars = variables(commonData).mapValues { (_, dfVar) -> commonData[dfVar] }
commonDfVars + ownDfVars
}

// ToDo: on client let's just 're-instert' all existing variables with name "@as_discrete@some_name"
if (isClientSide) {
val combinedDiscreteMappingVars = commonDiscreteMappings.variables() + ownDiscreteMappings.variables()

return Pair(
// no new discrete mappings, all job was done on server side
emptyMap<Any?, Any?>(),
// re-insert existing variables as discrete
combinedDfVars
.filter { (varName, _) -> varName in combinedDiscreteMappingVars }
.filter { (varName, _) -> isDiscrete(varName) }
.entries
.fold(DataFrame.Builder(data)) { acc, (varName, values) ->
val variable = findVariableOrFail(data, varName)
Expand All @@ -110,18 +96,26 @@ object DataMetaUtil {
)

} else { // server side
// Original (not encoded) discrete var names from both common and own mappings.
val combinedDiscreteMappingVars = mutableSetOf<String>()
val ownMappings = options.getMap(Option.PlotBase.MAPPING)

// own names - not yet encoded, i.e. 'cyl'
combinedDiscreteMappingVars += ownDiscreteMappings.variables()
val ownDiscreteMappings = run {
val ownDiscreteAes = getAsDiscreteAesSet(options.getMap(Option.Meta.DATA_META))
ownMappings.filter { (aes, _) -> aes in ownDiscreteAes }
}

// common names - already encoded by PlotConfig, i.e. '@as_discrete@cyl'. Restore original name.
combinedDiscreteMappingVars += commonDiscreteMappings.variables().map(::fromDiscrete)
val commonDiscreteVars = commonMapping.filterKeys { it in commonDiscreteAes }.variables().map(::fromDiscrete)

// Original (not encoded) discrete var names from both common and own mappings.
val combinedDiscreteMappingVars = mutableSetOf<String>()
combinedDiscreteMappingVars += ownDiscreteMappings.variables()
combinedDiscreteMappingVars += commonDiscreteVars
// minus own non-discrete mappings (layer overrides plot)
combinedDiscreteMappingVars -= ownMappings.variables() - ownDiscreteMappings.variables()

return Pair(
ownDiscreteMappings.mapValues { (_, varName) ->
ownMappings + ownDiscreteMappings.mapValues { (_, varName) ->
require(varName is String)
require(!isDiscrete(varName)) { "Already encoded discrete mapping: $varName" }
toDiscrete(varName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,18 +70,16 @@ class LayerConfig(
}

init {
val (discreteMappings, layerData) = createDataFrame(
val (layerMappings, layerData) = createDataFrame(
options = this,
commonData = sharedData,
commonDiscreteAes = plotDiscreteAes,
commonMapping = plotMapping,
isClientSide = myClientSide
)

// ToDo: isNotEmpty check is not necessary
// ToDo: do 'update' only on server side.
if (discreteMappings.isNotEmpty()) {
update(MAPPING, getMap(MAPPING) + discreteMappings)
if (!myClientSide) {
update(MAPPING, layerMappings)
}

// mapping (inherit from plot) + 'layer' mapping
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ abstract class PlotConfig(

init {

val (discreteMappings, plotData) = DataMetaUtil.createDataFrame(
val (plotMappings, plotData) = DataMetaUtil.createDataFrame(
options = this,
commonData = DataFrame.Builder.emptyFrame(),
commonDiscreteAes = emptySet(),
Expand All @@ -53,8 +53,8 @@ abstract class PlotConfig(

sharedData = plotData

if (discreteMappings.isNotEmpty()) {
update(MAPPING, getMap(MAPPING) + discreteMappings)
if (plotMappings.isNotEmpty()) {
update(MAPPING, plotMappings)
}

scaleConfigs = createScaleConfigs(getList(SCALES) + DataMetaUtil.createScaleSpecs(opts))
Expand Down
5 changes: 2 additions & 3 deletions python-package/lets_plot/mapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@ def __init__(self, variable, annotation):
if annotation is None:
raise ValueError("annotation can't be none")

# ToDo: rename name->variable, kind->annotation
self.name = variable
self.kind = annotation
self.variable = variable
self.annotation = annotation


def as_discrete(variable):
Expand Down
4 changes: 2 additions & 2 deletions python-package/lets_plot/plot/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ def as_annotated_data(raw_data: Any, raw_mapping: dict) -> Tuple:
continue

if isinstance(variable, MappingMeta):
mapping[aesthetic] = variable.name
mapping_meta.append({ 'aes': aesthetic, 'annotation': variable.kind })
mapping[aesthetic] = variable.variable
mapping_meta.append({ 'aes': aesthetic, 'annotation': variable.annotation})
else:
mapping[aesthetic] = variable

Expand Down