Fix #130, refactoring

JetBrains · May 8, 2020 · 7e58c36 · 7e58c36
1 parent c34ffac
commit 7e58c36
Show file tree

Hide file tree

Showing 15 changed files with 133 additions and 96 deletions.
diff --git a/...er-portable/src/commonMain/kotlin/jetbrains/datalore/plot/builder/map/GeoPositionField.kt b/...er-portable/src/commonMain/kotlin/jetbrains/datalore/plot/builder/map/GeoPositionField.kt
@@ -21,7 +21,4 @@ object GeoPositionField {
     const val RECT_XMAX = "lonmax"
     const val RECT_YMIN = "latmin"
     const val RECT_YMAX = "latmax"
-
-    // Key used for join from MAP
-    const val DATA_JOIN_KEY_COLUMN = "__key__"
-}
+}
diff --git a/...fig-portable/src/commonMain/kotlin/jetbrains/datalore/plot/config/GeoPositionsDataUtil.kt b/...fig-portable/src/commonMain/kotlin/jetbrains/datalore/plot/config/GeoPositionsDataUtil.kt
@@ -14,25 +14,13 @@ import jetbrains.datalore.plot.base.GeomKind
 import jetbrains.datalore.plot.base.data.DataFrameUtil
 import jetbrains.datalore.plot.builder.map.GeoPositionField
 import jetbrains.datalore.plot.config.Option.Geom.Choropleth
+import jetbrains.datalore.plot.config.Option.Meta.MapJoin
 
 object GeoPositionsDataUtil {
-    // Auto-generated by GeoPositionMappingChange
-    const val MAP_JOIN_ID_COLUMN = "key"
     // Provided by regions object
     const val MAP_REGION_COLUMN = "region"
 
     const val MAP_OSM_ID_COLUMN = "__geoid__"
-    const val MAP_GEOMETRY_COLUMN = "__geometry__"
-
-    const val MAP_JOIN = "map_join"
-
-    // Column in map to be used in JOIN with data
-    const val MAP_JOIN_COLUMN = "map_join_column"
-
-    // Columns can be used as request
-    val GEO_POSITIONS_KEYS = listOf(
-        MAP_JOIN_ID_COLUMN
-    )
 
     private val GEOMS_SUPPORT = mapOf(
             GeomKind.MAP to GeoDataSupport.boundary(),
@@ -108,15 +96,15 @@ object GeoPositionsDataUtil {
     }
 
     private fun getGeoPositionsIdVar(mapOptions: DataFrame): Variable {
-        val variable = findFirstVariable(mapOptions, listOf(MAP_JOIN_ID_COLUMN, MAP_REGION_COLUMN))
+        val variable = findFirstVariable(mapOptions, listOf(MapJoin.ID, MAP_REGION_COLUMN))
         if (variable != null) {
             return variable
         }
 
         throw IllegalArgumentException(
             geoPositionsColumnNotFoundError(
                 "region id",
-                listOf(MAP_JOIN_ID_COLUMN, MAP_REGION_COLUMN)
+                listOf(MapJoin.ID, MAP_REGION_COLUMN)
             )
         )
     }

diff --git a/plot-config-portable/src/commonMain/kotlin/jetbrains/datalore/plot/config/Option.kt b/plot-config-portable/src/commonMain/kotlin/jetbrains/datalore/plot/config/Option.kt
@@ -30,15 +30,29 @@ object Option {
 
         object GeoDataFrame {
             const val TAG = "geodataframe"
-            const val GEOMETRY = "geometry"
+            const val GEOMETRY_COLUMN_NAME = "geometry"
+
+            // Column with geometries extracted from GeoDataFrame. Can be used either in DATA and MAP
+            const val GEOMETRIES = "__geometry__"
         }
 
         object GeoReference {
             const val TAG = "georeference"
             const val REQUEST = "request"
+            const val MAP_REGION_COLUMN = "region"
             const val OSM_ID = "id"
         }
 
+        object GeoDict {
+            const val TAG = "geodict"
+        }
+
+        object MapJoin {
+            // Column with keys used for join
+            const val ID = "__id__"
+            const val MAP_JOIN_COLUMN = "__map_join_column__"
+        }
+
         object MappingAnnotation {
             const val TAG = "mapping_annotations"
             const val AES = "aes"
@@ -84,6 +98,7 @@ object Option {
         const val SAMPLING = "sampling"
         const val SHOW_LEGEND = "show_legend"
         const val TOOLTIP = "tooltip"
+        const val MAP_JOIN = "map_join"
     }
 
     object Geom {

diff --git a/...portable/src/commonMain/kotlin/jetbrains/datalore/plot/config/PlotConfigClientSideUtil.kt b/...portable/src/commonMain/kotlin/jetbrains/datalore/plot/config/PlotConfigClientSideUtil.kt
@@ -18,8 +18,8 @@ import jetbrains.datalore.plot.builder.assemble.TypedScaleProviderMap
 import jetbrains.datalore.plot.builder.interact.GeomInteractionBuilder
 import jetbrains.datalore.plot.builder.interact.GeomInteractionBuilder.Companion.AREA_GEOM
 import jetbrains.datalore.plot.builder.interact.GeomInteractionBuilder.Companion.NON_AREA_GEOM
-import jetbrains.datalore.plot.builder.map.GeoPositionField
 import jetbrains.datalore.plot.builder.theme.Theme
+import jetbrains.datalore.plot.config.Option.Meta.MapJoin
 
 object PlotConfigClientSideUtil {
     internal fun createGuideOptionsMap(scaleConfigs: List<ScaleConfig<*>>): Map<Aes<*>, GuideOptions> {
@@ -251,7 +251,7 @@ object PlotConfigClientSideUtil {
         }
 
         // remove auto generated mappings
-        aesListForTooltip.removeAll { layerConfig.getScaleForAes(it)?.name == GeoPositionField.DATA_JOIN_KEY_COLUMN }
+        aesListForTooltip.removeAll { layerConfig.getScaleForAes(it)?.name == MapJoin.ID }
 
         // remove map_id mapping
         aesListForTooltip.removeAll { it === Aes.MAP_ID }

diff --git a/...rc/commonMain/kotlin/jetbrains/datalore/plot/config/geo/GeometryFromGeoDataFrameChange.kt b/...rc/commonMain/kotlin/jetbrains/datalore/plot/config/geo/GeometryFromGeoDataFrameChange.kt
@@ -14,9 +14,10 @@ import jetbrains.datalore.plot.builder.map.GeoPositionField.RECT_XMIN
 import jetbrains.datalore.plot.builder.map.GeoPositionField.RECT_YMAX
 import jetbrains.datalore.plot.builder.map.GeoPositionField.RECT_YMIN
 import jetbrains.datalore.plot.config.GeoPositionsDataUtil.GeoDataKind
-import jetbrains.datalore.plot.config.GeoPositionsDataUtil.MAP_GEOMETRY_COLUMN
-import jetbrains.datalore.plot.config.GeoPositionsDataUtil.MAP_JOIN_ID_COLUMN
 import jetbrains.datalore.plot.config.Option
+import jetbrains.datalore.plot.config.Option.Meta.GeoDataFrame
+import jetbrains.datalore.plot.config.Option.Meta.MapJoin
+import jetbrains.datalore.plot.config.getList
 import jetbrains.datalore.plot.config.transform.SpecSelector
 
 
@@ -25,7 +26,7 @@ class GeometryFromGeoDataFrameChange : GeometryFromGeoPositionsChange() {
         get() = GEO_DATA_FRAME_KEYS
 
     override fun changeGeoPositions(mapSpec: MutableMap<String, Any>, geoDataKind: GeoDataKind) {
-        val geometryTables = (mapSpec[MAP_GEOMETRY_COLUMN] as List<*>).map { parseGeometry(it as String, geoDataKind) }
+        val geometryTables = mapSpec.getList(GeoDataFrame.GEOMETRIES)!!.map { parseGeometry(it as String, geoDataKind) }
 
         if (geometryTables.sumBy { it.rowCount } == 0) {
             error(
@@ -39,12 +40,12 @@ class GeometryFromGeoDataFrameChange : GeometryFromGeoPositionsChange() {
             )
         }
 
-        val dataTable = (mapSpec[MAP_JOIN_ID_COLUMN] as? List<*>)
+        val dataTable = mapSpec.getList(MapJoin.ID)
             ?.zip(geometryTables)
             ?.fold(mutableMapOf<String, MutableList<Any>>(), { dataFrame, (id, geometryTable) ->
                 dataFrame
                     .concat(geometryTable)
-                    .concat(MAP_JOIN_ID_COLUMN, MutableList(geometryTable.rowCount) { id as String })
+                    .concat(MapJoin.ID, MutableList(geometryTable.rowCount) { id!! })
             })
             ?: geometryTables.fold(mutableMapOf<String, MutableList<Any>>(), { dataFrame, geometryTable ->
                 dataFrame.concat(geometryTable)
@@ -103,7 +104,7 @@ class GeometryFromGeoDataFrameChange : GeometryFromGeoPositionsChange() {
             }
         )
 
-        private val GEO_DATA_FRAME_KEYS: Set<String> = setOf(MAP_GEOMETRY_COLUMN)
+        private val GEO_DATA_FRAME_KEYS: Set<String> = setOf(GeoDataFrame.GEOMETRIES)
 
         internal fun defaultConsumer(config: SimpleFeature.Consumer<LonLat>.() -> Unit) =
             SimpleFeature.Consumer<LonLat>(
@@ -123,8 +124,8 @@ private fun <T> MutableMap<String, MutableList<T>>.concat(other: Map<String, Lis
     other.forEach { (key, value) -> getOrPut(key, { ArrayList() }).addAll(value) }
 }
 
-private fun <T> MutableMap<String, MutableList<T>>.concat(key: String, values: List<T>) = apply {
-    concat(mutableMapOf(key to values))
+private fun <T> MutableMap<String, MutableList<T>>.concat(column: String, values: List<T>) = apply {
+    concat(mutableMapOf(column to values))
 }
 
 private fun MutableMap<String, MutableList<Double>>.append(key: String, value: Double) {

diff --git a/...rc/commonMain/kotlin/jetbrains/datalore/plot/config/geo/GeometryFromGeoPositionsChange.kt b/...rc/commonMain/kotlin/jetbrains/datalore/plot/config/geo/GeometryFromGeoPositionsChange.kt
@@ -12,6 +12,8 @@ import jetbrains.datalore.plot.config.GeoPositionsDataUtil.isGeomSupported
 import jetbrains.datalore.plot.config.Option
 import jetbrains.datalore.plot.config.Option.Geom.Choropleth.GEO_POSITIONS
 import jetbrains.datalore.plot.config.Option.Layer.GEOM
+import jetbrains.datalore.plot.config.asMutable
+import jetbrains.datalore.plot.config.getMap
 import jetbrains.datalore.plot.config.transform.SpecChange
 import jetbrains.datalore.plot.config.transform.SpecChangeContext
 
@@ -28,10 +30,10 @@ abstract class GeometryFromGeoPositionsChange : SpecChange {
     }
 
     override fun apply(spec: MutableMap<String, Any>, ctx: SpecChangeContext) {
-        @Suppress("UNCHECKED_CAST")
-        val mapSpec = spec[GEO_POSITIONS] as MutableMap<String, Any>
-        val geoDataKind: GeoPositionsDataUtil.GeoDataKind = getGeoDataKind(getGeomKind(spec))
-        changeGeoPositions(mapSpec, geoDataKind)
+        changeGeoPositions(
+            mapSpec = spec.getMap(GEO_POSITIONS)!!.asMutable(),
+            geoDataKind = getGeoDataKind(getGeomKind(spec))
+        )
     }
 
     abstract val geoPositionsKeys: Set<String>

diff --git a/...nMain/kotlin/jetbrains/datalore/plot/server/config/transform/GeoDataFrameMappingChange.kt b/...nMain/kotlin/jetbrains/datalore/plot/server/config/transform/GeoDataFrameMappingChange.kt
@@ -5,15 +5,13 @@
 
 package jetbrains.datalore.plot.server.config.transform
 
-import jetbrains.datalore.plot.builder.map.GeoPositionField.DATA_JOIN_KEY_COLUMN
 import jetbrains.datalore.plot.config.*
-import jetbrains.datalore.plot.config.GeoPositionsDataUtil.MAP_GEOMETRY_COLUMN
-import jetbrains.datalore.plot.config.GeoPositionsDataUtil.MAP_JOIN_ID_COLUMN
 import jetbrains.datalore.plot.config.Option.Geom.Choropleth.GEO_POSITIONS
 import jetbrains.datalore.plot.config.Option.Mapping.MAP_ID
 import jetbrains.datalore.plot.config.Option.Meta.DATA_META
 import jetbrains.datalore.plot.config.Option.Meta.GeoDataFrame
-import jetbrains.datalore.plot.config.Option.Meta.GeoDataFrame.GEOMETRY
+import jetbrains.datalore.plot.config.Option.Meta.GeoDataFrame.GEOMETRY_COLUMN_NAME
+import jetbrains.datalore.plot.config.Option.Meta.MapJoin
 import jetbrains.datalore.plot.config.Option.Plot
 import jetbrains.datalore.plot.config.Option.PlotBase.DATA
 import jetbrains.datalore.plot.config.Option.PlotBase.MAPPING
@@ -24,15 +22,15 @@ import jetbrains.datalore.plot.config.transform.SpecSelector
 class GeoDataFrameMappingChange : SpecChange {
 
     override fun apply(spec: MutableMap<String, Any>, ctx: SpecChangeContext) {
-        val geometryColumnName = spec.read(DATA_META, GeoDataFrame.TAG, GEOMETRY) as String
+        val geometryColumnName = spec.read(DATA_META, GeoDataFrame.TAG, GEOMETRY_COLUMN_NAME) as String
         val geometries = spec.getList(DATA, geometryColumnName)!!
-        val keys = geometries.indices.map(Int::toString)
+        val ids = geometries.indices.map(Int::toString)
 
         spec.remove(DATA, geometryColumnName)
-        spec.write(DATA, DATA_JOIN_KEY_COLUMN) { keys }
-        spec.write(GEO_POSITIONS, MAP_JOIN_ID_COLUMN) { keys }
-        spec.write(GEO_POSITIONS, MAP_GEOMETRY_COLUMN) { geometries}
-        spec.write(MAPPING, MAP_ID) { DATA_JOIN_KEY_COLUMN }
+        spec.write(DATA, MapJoin.ID) { ids }
+        spec.write(GEO_POSITIONS, MapJoin.ID) { ids }
+        spec.write(GEO_POSITIONS, GeoDataFrame.GEOMETRIES) { geometries}
+        spec.write(MAPPING, MAP_ID) { MapJoin.ID }
     }
 
     override fun isApplicable(spec: Map<String, Any>): Boolean {

diff --git a/...onMain/kotlin/jetbrains/datalore/plot/server/config/transform/GeoPositionMappingChange.kt b/...onMain/kotlin/jetbrains/datalore/plot/server/config/transform/GeoPositionMappingChange.kt
@@ -5,51 +5,54 @@
 
 package jetbrains.datalore.plot.server.config.transform
 
-import jetbrains.datalore.plot.config.GeoPositionsDataUtil.MAP_GEOMETRY_COLUMN
-import jetbrains.datalore.plot.config.GeoPositionsDataUtil.MAP_JOIN_COLUMN
-import jetbrains.datalore.plot.config.GeoPositionsDataUtil.MAP_JOIN_ID_COLUMN
+import jetbrains.datalore.plot.config.*
 import jetbrains.datalore.plot.config.GeoPositionsDataUtil.MAP_OSM_ID_COLUMN
 import jetbrains.datalore.plot.config.GeoPositionsDataUtil.MAP_REGION_COLUMN
 import jetbrains.datalore.plot.config.Option.Geom.Choropleth.GEO_POSITIONS
 import jetbrains.datalore.plot.config.Option.Meta.GeoDataFrame
-import jetbrains.datalore.plot.config.Option.Meta.GeoDataFrame.GEOMETRY
+import jetbrains.datalore.plot.config.Option.Meta.GeoDataFrame.GEOMETRY_COLUMN_NAME
+import jetbrains.datalore.plot.config.Option.Meta.GeoDict
 import jetbrains.datalore.plot.config.Option.Meta.GeoReference
 import jetbrains.datalore.plot.config.Option.Meta.MAP_DATA_META
+import jetbrains.datalore.plot.config.Option.Meta.MapJoin
 import jetbrains.datalore.plot.config.Option.Plot
-import jetbrains.datalore.plot.config.getMap
-import jetbrains.datalore.plot.config.has
-import jetbrains.datalore.plot.config.read
 import jetbrains.datalore.plot.config.transform.SpecChange
 import jetbrains.datalore.plot.config.transform.SpecChangeContext
 import jetbrains.datalore.plot.config.transform.SpecSelector
-import jetbrains.datalore.plot.config.write
 
 class GeoPositionMappingChange : SpecChange {
 
     override fun apply(spec: MutableMap<String, Any>, ctx: SpecChangeContext) {
         val mapSpec = spec.getMap(GEO_POSITIONS)!!
-        spec.read(MAP_DATA_META, GeoDataFrame.TAG, GEOMETRY)
-            ?.let {it as String }
-            ?.let { geometry ->
-                mapSpec.write(MAP_GEOMETRY_COLUMN) { mapSpec.read(geometry)!! }
-
-                listOfNotNull(
-                    spec.read(MAP_JOIN_COLUMN)?.let { mapSpec.read(it as String) },
-                    mapSpec.read(GeoReference.REQUEST),
-                    mapSpec.read(MAP_JOIN_ID_COLUMN),
-                    mapSpec.read(MAP_REGION_COLUMN)
-                ).firstOrNull()?.let { mapSpec.write(MAP_JOIN_ID_COLUMN) { it } }
-            }
-
-        spec.read(MAP_DATA_META, GeoReference.TAG)?.run {
-            mapSpec.write(MAP_JOIN_ID_COLUMN) { mapSpec.read(GeoReference.REQUEST)!! }
+        val mapJoinIds = spec.read(MapJoin.MAP_JOIN_COLUMN)?.let { mapSpec.read(it as String) }
+
+        if (spec.has(MAP_DATA_META, GeoDataFrame.TAG)) {
+            // Select column applicable for join
+            listOfNotNull(
+                mapJoinIds, // user defined column via parameter `map_join`
+                mapSpec.read(GeoReference.REQUEST), // Regions object from our geocoding
+                mapSpec.read(MAP_REGION_COLUMN) // ???
+            ).firstOrNull()?.let { mapSpec.write(MapJoin.ID) { it } }
+
+            spec.read(MAP_DATA_META, GeoDataFrame.TAG, GEOMETRY_COLUMN_NAME)
+                ?.let { geometryColumnName -> mapSpec.read(geometryColumnName as String)!! }
+                ?.let { geometries -> mapSpec.write(GeoDataFrame.GEOMETRIES) { geometries } }
+        }
+
+        if (spec.has(MAP_DATA_META, GeoReference.TAG)) {
+            mapSpec.write(MapJoin.ID) { mapSpec.read(GeoReference.REQUEST)!! }
             mapSpec.write(MAP_OSM_ID_COLUMN) { mapSpec.read(GeoReference.OSM_ID)!! }
         }
+
+        if (spec.has(MAP_DATA_META, GeoDict.TAG)) {
+            mapJoinIds?.let { mapSpec.write(MapJoin.ID) { it } }
+        }
     }
 
     override fun isApplicable(spec: Map<String, Any>): Boolean {
         return spec.has(MAP_DATA_META, GeoDataFrame.TAG) ||
-                spec.has(MAP_DATA_META, GeoReference.TAG)
+                spec.has(MAP_DATA_META, GeoReference.TAG) ||
+                spec.has(MAP_DATA_META, GeoDict.TAG)
     }
 
     companion object {

diff --git a/...le/src/commonMain/kotlin/jetbrains/datalore/plot/server/config/transform/MapJoinChange.kt b/...le/src/commonMain/kotlin/jetbrains/datalore/plot/server/config/transform/MapJoinChange.kt
@@ -5,11 +5,11 @@
 
 package jetbrains.datalore.plot.server.config.transform
 
-import jetbrains.datalore.plot.config.GeoPositionsDataUtil.MAP_JOIN
-import jetbrains.datalore.plot.config.GeoPositionsDataUtil.MAP_JOIN_COLUMN
 import jetbrains.datalore.plot.config.Option
+import jetbrains.datalore.plot.config.Option.Layer.MAP_JOIN
 import jetbrains.datalore.plot.config.Option.Mapping.MAP_ID
 import jetbrains.datalore.plot.config.Option.Meta.MAP_DATA_META
+import jetbrains.datalore.plot.config.Option.Meta.MapJoin
 import jetbrains.datalore.plot.config.Option.PlotBase.MAPPING
 import jetbrains.datalore.plot.config.getList
 import jetbrains.datalore.plot.config.transform.SpecChange
@@ -21,7 +21,7 @@ class MapJoinChange: SpecChange {
     override fun apply(spec: MutableMap<String, Any>, ctx: SpecChangeContext) {
         val (dataJoinColumn, mapJoinColumn) = spec.getList(MAP_JOIN)!!
         dataJoinColumn?.let { spec.write(MAPPING, MAP_ID) { it } }
-        mapJoinColumn?.let { spec.write(MAP_JOIN_COLUMN) { it } }
+        mapJoinColumn?.let { spec.write(MapJoin.MAP_JOIN_COLUMN) { it } }
     }
 
     override fun isApplicable(spec: Map<String, Any>): Boolean {

diff --git a/plot-config-portable/src/commonTest/kotlin/plot/config/GeomInteractionBuilderCreationTest.kt b/plot-config-portable/src/commonTest/kotlin/plot/config/GeomInteractionBuilderCreationTest.kt
@@ -6,9 +6,9 @@
 package plot.config
 
 import jetbrains.datalore.plot.base.Aes
-import jetbrains.datalore.plot.builder.map.GeoPositionField
 import jetbrains.datalore.plot.config.LayerConfig
 import jetbrains.datalore.plot.config.Option
+import jetbrains.datalore.plot.config.Option.Meta.MapJoin
 import jetbrains.datalore.plot.config.Option.PlotBase.DATA
 import jetbrains.datalore.plot.config.Option.PlotBase.MAPPING
 import jetbrains.datalore.plot.config.PlotConfigClientSideUtil
@@ -167,7 +167,7 @@ class GeomInteractionBuilderCreationTest {
         )
         val GEO_DATA_FRAME_META = mapOf(
             Option.Meta.GeoDataFrame.TAG to mapOf(
-                Option.Meta.GeoDataFrame.GEOMETRY to "coord"
+                Option.Meta.GeoDataFrame.GEOMETRY_COLUMN_NAME to "coord"
             )
         )
         val plotOpts = mutableMapOf(
@@ -183,7 +183,7 @@ class GeomInteractionBuilderCreationTest {
         )
         val layerConfig = createLayerConfig(plotOpts)
 
-        val binding = layerConfig.varBindings.find { it.variable.name == GeoPositionField.DATA_JOIN_KEY_COLUMN }
+        val binding = layerConfig.varBindings.find { it.variable.name == MapJoin.ID }
         assertNotNull(binding)
 
         val builder = PlotConfigClientSideUtil.createGeomInteractionBuilder(