Skip to content

Commit

Permalink
Fix #1186 - Display integer values without fractional part in tooltips
Browse files Browse the repository at this point in the history
  • Loading branch information
IKupriyanov-HORIS committed Nov 28, 2024
1 parent 2a28249 commit a5ecb03
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 13 deletions.
3 changes: 2 additions & 1 deletion future_changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@
- Wrong formatting when type='s' with explicit precision [[#1240](https://github.com/JetBrains/lets-plot/issues/1240)].
- Extra trim in formatted number when type='g' [[#1241](https://github.com/JetBrains/lets-plot/issues/1241)].
- Axis breaks are badly formatted if explicitly set [[#1245](https://github.com/JetBrains/lets-plot/issues/1245)].
- Badly formatted zero break for the "~g" format [[#1246](https://github.com/JetBrains/lets-plot/issues/1246)].
- Badly formatted zero break for the "~g" format [[#1246](https://github.com/JetBrains/lets-plot/issues/1246)].
- Display integer values without fractional part in tooltips [[#1186](https://github.com/JetBrains/lets-plot/issues/1186)].
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,6 @@ interface MappedDataAccess {
// fun getMappedDataValue(aes: Aes<*>, index: Int, ctx: PlotContext): String

fun getMappedDataLabel(aes: Aes<*>): String

val defaultFormatters: Map<Any, (Any) -> String>
}
Original file line number Diff line number Diff line change
Expand Up @@ -365,13 +365,13 @@ class GeomLayerBuilder(
}

override fun createContextualMapping(): ContextualMapping {
val dataAccess = PointDataAccess(dataFrame, varBindings, scaleMap, isYOrientation)
val dataAccess = PointDataAccess(dataFrame, varBindings, scaleMap, isYOrientation, defaultFormatters)
return contextualMappingProvider.createContextualMapping(dataAccess, dataFrame)
}

override fun createAnnotation(): Annotation? {
return annotationProvider?.let { provider ->
val dataAccess = PointDataAccess(dataFrame, varBindings, scaleMap, isYOrientation)
val dataAccess = PointDataAccess(dataFrame, varBindings, scaleMap, isYOrientation, defaultFormatters)
provider(dataAccess, dataFrame)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,10 @@ internal class PointDataAccess(
private val data: DataFrame,
private val bindings: Map<Aes<*>, VarBinding>,
private val scaleMap: Map<Aes<*>, Scale>,
override val isYOrientation: Boolean
override val isYOrientation: Boolean,
override val defaultFormatters: Map<Any, (Any) -> String>
) : MappedDataAccess {

private val myFormatters = HashMap<Aes<*>, (Any?) -> String>()

override fun isMapped(aes: Aes<*>) = bindings.containsKey(aes)

override fun getOriginalValue(aes: Aes<*>, index: Int): Any? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package org.jetbrains.letsPlot.core.plot.builder.tooltip

import org.jetbrains.letsPlot.commons.formatting.string.StringFormat
import org.jetbrains.letsPlot.core.commons.data.DataType
import org.jetbrains.letsPlot.core.plot.base.Aes
import org.jetbrains.letsPlot.core.plot.base.DataFrame
import org.jetbrains.letsPlot.core.plot.base.PlotContext
Expand All @@ -30,13 +31,11 @@ internal object TooltipFormatting {
}
}

fun createFormatter(variable: DataFrame.Variable, expFormat: StringFormat.ExponentFormat): (Any) -> String {
fun createFormatter(variable: DataFrame.Variable, formatters: Map<Any, (Any) -> String>, expFormat: StringFormat.ExponentFormat): (Any) -> String {
return when (variable) {
Stats.PROP,
Stats.SUMPROP -> StringFormat.forOneArg(".2f", formatFor = variable.name, expFormat = expFormat)::format
Stats.PROPPCT,
Stats.SUMPCT -> StringFormat.forOneArg("{.1f} %", formatFor = variable.name, expFormat = expFormat)::format
else -> { value -> value.toString() }
Stats.PROP, Stats.SUMPROP -> StringFormat.forOneArg(".2f", formatFor = variable.name, expFormat = expFormat)::format
Stats.PROPPCT, Stats.SUMPCT -> StringFormat.forOneArg("{.1f} %", formatFor = variable.name, expFormat = expFormat)::format
else -> formatters[variable.name] ?: DataType.UNKNOWN.formatter
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class DataFrameField(
private val format: String? = null
) : ValueSource {

private lateinit var myDataAccess: MappedDataAccess
private lateinit var myDataFrame: DataFrame
private lateinit var myVariable: DataFrame.Variable
private var myFormatter: ((Any) -> String)? = null
Expand All @@ -26,7 +27,7 @@ class DataFrameField(
require(myFormatter == null)

myFormatter = when (format) {
null -> TooltipFormatting.createFormatter(myVariable, expFormat)
null -> TooltipFormatting.createFormatter(myVariable, myDataAccess.defaultFormatters, expFormat)
else -> StringFormat.forOneArg(format, formatFor = name, expFormat = expFormat)::format
}
return myFormatter!!
Expand All @@ -37,6 +38,9 @@ class DataFrameField(
override val isAxis: Boolean = false

override fun initDataContext(data: DataFrame, mappedDataAccess: MappedDataAccess) {
require(!::myDataAccess.isInitialized) { "Data context can be initialized only once" }
myDataAccess = mappedDataAccess

require(!::myDataFrame.isInitialized) { "Data context can be initialized only once" }

myDataFrame = data
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,4 +235,42 @@ class TooltipCheckLabelInLines {
expectedLines = listOf("b: 10")
)
}

@Test
fun `issue 1186 - dataframe variable should be formatted using the DataType from series_annotations`() {
val spec = """
|{
| "data": {
| "l": [ 3.0 ],
| "b": [ 4.0 ]
| },
| "mapping": { "color": "l" },
| "data_meta": {
| "series_annotations": [
| { "type": "int", "column": "l" },
| { "type": "int", "column": "b" }
| ]
| },
| "ggsize": { "width": 300.0, "height": 200.0 },
| "kind": "plot",
| "layers": [
| {
| "geom": "point",
| "tooltips": {
| "lines": [
| "l is @l",
| "b is @b"
| ]
| }
| }
| ]
|}
""".trimMargin()

val layer = TestingGeomLayersBuilder.getSingleGeomLayer(spec)
assertGeneralTooltip(
layer,
expectedLines = listOf("l is 3", "b is 4")
)
}
}

0 comments on commit a5ecb03

Please sign in to comment.