Skip to content

Commit

Permalink
Fix #634 : Precision error in gradient.
Browse files Browse the repository at this point in the history
  • Loading branch information
alshan committed Nov 30, 2022
1 parent 9517b56 commit 346d09b
Show file tree
Hide file tree
Showing 7 changed files with 134 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@ object Transforms {
return BreaksGeneratorForTransformedDomain(transform, breaksGenerator)
}

/**
* Use with caution!
*
* Do not use this method on transformed data ranges. (see 'SeriesUtil.ensureApplicableRange()')
*
* Only use on original data ranges.
*/
fun ensureApplicableDomain(
dataRange: DoubleSpan?,
transform: ContinuousTransform
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import jetbrains.datalore.plot.base.aes.AestheticsDefaults
import jetbrains.datalore.plot.base.data.TransformVar
import jetbrains.datalore.plot.base.render.LegendKeyElementFactory
import jetbrains.datalore.plot.base.scale.ScaleUtil
import jetbrains.datalore.plot.base.scale.transform.Transforms
import jetbrains.datalore.plot.builder.GeomLayer
import jetbrains.datalore.plot.common.data.SeriesUtil

internal class PlotAssemblerPlotContext(
layersByTile: List<List<GeomLayer>>,
Expand Down Expand Up @@ -99,24 +99,24 @@ internal class PlotAssemblerPlotContext(
}
}

val overallDomainRaw = domainsRaw.reduceOrNull { acc, v -> acc.union(v) }
val overallTransformedDomain = domainsRaw.reduceOrNull { acc, v -> acc.union(v) }

val scale = scaleMap.get(aes)
return if (scale.isContinuousDomain) {
finalizeOverallTransformedDomain(overallDomainRaw, scale.transform as ContinuousTransform)
finalizeOverallTransformedDomain(overallTransformedDomain, scale.transform as ContinuousTransform)
} else {
// Discrete domain
overallDomainRaw ?: DoubleSpan.singleton(0.0)
overallTransformedDomain ?: DoubleSpan.singleton(0.0)
}
}

private fun finalizeOverallTransformedDomain(
domain: DoubleSpan?,
transformedDomain: DoubleSpan?,
transform: ContinuousTransform
): DoubleSpan {
val (dataLower, dataUpper) = when (domain) {
val (dataLower, dataUpper) = when (transformedDomain) {
null -> Pair(Double.NaN, Double.NaN)
else -> Pair(domain.lowerEnd, domain.upperEnd)
else -> Pair(transformedDomain.lowerEnd, transformedDomain.upperEnd)
}
val (scaleLower, scaleUpper) = ScaleUtil.transformedDefinedLimits(transform)

Expand All @@ -130,7 +130,7 @@ internal class PlotAssemblerPlotContext(
else -> null
}

return Transforms.ensureApplicableDomain(newRange, transform)
return SeriesUtil.ensureApplicableRange(newRange)
}

fun checkPositionalAes(aes: Aes<*>) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,36 +43,6 @@ class ColorGradientnMapperProvider(


companion object {
// internal fun createGradient(
// domain: DoubleSpan,
// colors: List<Color>,
// naValue: Color,
// alpha: Double = 1.0
// ): (Double?) -> Color {
// val subdomainsCount = colors.size - 1
// val subdomainLength = domain.length / subdomainsCount
//
// val mappers = (0..subdomainsCount)
// .map { domain.lowerEnd + subdomainLength * it }
// .zip(colors)
// .windowed(2)
// .map { (low, high) ->
// val (lowValue, lowColor) = low
// val (highValue, highColor) = high
// val subdomain = DoubleSpan(lowValue, highValue)
// subdomain to ColorMapper.gradient(subdomain, lowColor, highColor, naValue, alpha)
// }
//
// return { value ->
// value?.let {
// mappers
// .firstOrNull { (subdomain, _) -> value in subdomain }
// ?.let { (_, gradient) -> gradient(value) }
// ?: naValue
// } ?: naValue
// }
// }

internal fun createGradient(
domain: DoubleSpan,
colors: List<Color>,
Expand All @@ -82,7 +52,9 @@ class ColorGradientnMapperProvider(
val subdomainsCount = colors.size - 1
val subdomainLength = domain.length / subdomainsCount

val subdomainEnds = (0..subdomainsCount).map { domain.lowerEnd + subdomainLength * it }
val subdomainEnds = (0 until subdomainsCount)
.map { domain.lowerEnd + subdomainLength * it } +
listOf(domain.upperEnd) // The last "end" should be exact.
val mappers = subdomainEnds.zip(colors)
.windowed(2)
.map { (low, high) ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,12 @@ object SeriesUtil {
}

/**
* ToDo: Use with caution.
* ToDo: The correct method of domain validation is temporarily in 'Transforms.ensureApplicableDomain'.
* Use with caution!
*
* Do not use this method on original data ranges (i.e. before transform).
* The correct method for validation of original data ranges is 'Transforms.ensureApplicableDomain'.
*
* Can only be used on transformed data ranges or when "transform" is irrelevant.
*/
fun ensureApplicableRange(
range: DoubleSpan?,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,8 @@ class TooltipAxisConfigTest {

run {
val geomLayer = log10(scaleFormat = null, tooltipFormat = null)
assertGeneralTooltip(geomLayer, "0.34")
assertYAxisTooltip(geomLayer, "0.34")
assertGeneralTooltip(geomLayer, "0.344")
assertYAxisTooltip(geomLayer, "0.344")
assertEquals("0.32", getYTick(geomLayer, closedRange))
}
run {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/*
* Copyright (c) 2022. JetBrains s.r.o.
* Use of this source code is governed by the MIT license that can be found in the LICENSE file.
*/

package jetbrains.datalore.plotDemo.model.plotConfig

import jetbrains.datalore.plot.parsePlotSpec

/**
* https://github.com/JetBrains/lets-plot/issues/634
*/
@Suppress("ClassName")
class Issue_gradientn_634 {
fun plotSpecList(): List<MutableMap<String, Any>> {
return listOf(
case1(),
case2(),
)
}

private fun case1(): MutableMap<String, Any> {
// The "fill" shouldn't be black

val spec = """
{
'mapping' : {},
'data' : {
"x" : ["first", "first", "second", "third", "third"],
"y" : [18, 21, 21, 18, 21],
"z" : [3.5264934876244764E-7, 0.0974772170888232, 0.1116666955828674, 2.58687631451427E-5, 0.07896070396295593]},
"kind" : "plot",
"scales" : [
{
"aesthetic" : "fill",
"scale_mapper_kind" : "color_gradientn",
"na_value" : "#000000",
"colors" : [
"#DFFADC", "#C9F5D3", "#B3F2CF", "#9AEBCD", "#80DCCC",
"#6DC8D2", "#61B7DB", "#5C97DB", "#5A7CD6", "#6060C7",
"#674BB3", "#693799", "#6A277B", "#671D60", "#611347"]
}],
"layers" : [
{
"mapping" : {"x" : "x", "y" : "y", "fill" : "z"},
"geom" : "tile"
}]
}
""".trimIndent()

return parsePlotSpec(spec)
}

private fun case2(): MutableMap<String, Any> {
// The "fill" shouldn't be black

val spec = """
{
'mapping' : {},
'data' : {
'x' : ['first', 'first', 'second', 'third', 'third'],
'y' : [18, 21, 21, 18, 21],
'z' : [ 3.5264934876244764E-7,
0.0974772170888232,
0.1116666955828674,
2.58687631451427E-5,
0.5]},
'kind' : 'plot',
'scales' : [
{
'aesthetic' : 'fill',
'scale_mapper_kind' : 'color_gradientn',
'na_value' : '#000000',
'trans' : 'log10',
'colors' : [
'#DFFADC', '#C9F5D3', '#B3F2CF', '#9AEBCD', '#80DCCC',
'#6DC8D2', '#61B7DB', '#5C97DB', '#5A7CD6', '#6060C7',
'#674BB3', '#693799', '#6A277B', '#671D60', '#611347']
}],
'layers' : [
{
'mapping' : {'x' : 'x', 'y' : 'y', 'fill' : 'z'},
'geom' : 'tile'
}]
}
""".trimIndent()

return parsePlotSpec(spec)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* Copyright (c) 2022. JetBrains s.r.o.
* Use of this source code is governed by the MIT license that can be found in the LICENSE file.
*/

package jetbrains.datalore.plotDemo.plotConfig

import jetbrains.datalore.plotDemo.model.plotConfig.Issue_gradientn_634
import jetbrains.datalore.vis.demoUtils.PlotSpecsDemoWindowBatik

fun main() {
with(Issue_gradientn_634()) {
PlotSpecsDemoWindowBatik(
"Issue_gradientn_634",
plotSpecList()
).open()
}
}

0 comments on commit 346d09b

Please sign in to comment.