JetBrains / lets-plot

Multiplatform plotting library based on the Grammar of Graphics
https://lets-plot.org
MIT License
1.57k stars 51 forks source link

scaleColorManual swaps labels if dataframe contains single label #507

Closed ccjernigan closed 2 years ago

ccjernigan commented 2 years ago

STEPS TO REPRODUCE

  1. Create a dataframe with a single label (e.g. "sedentary")
  2. Create a scaleColorManual with multiple labels (e.g. "sedentary", "walking", "running", "biking")
  3. Graph the data

RESULTS Actual The legend shows "walking" with the color for walking, but there was no walking data in the dataframe. The sedentary data was erroneously remapped to walking.

Expected The legend shows "sedentary" with the color for sedentary.

NOTES If I remove scaleColorManual, the bug no longer occurs.

I'm graphing wearable sensor data with a set of different labels. Not every label will be in every graph, and some graphs might only have a single label.

As an example, consider graphing heart rate versus activities like sedentary, walking, running, biking. In some instances, a user might be sedentary the entire time and so the other labels won't appear. When a graph contains at least 2 labels, the colors are correct. But when it contains a single label, then the color of that single label can be wrong. I believe there might be an ordering component to trigger this issue.

In terms of the actual API usage, my usage looks something like this:

val plotData: Map<String, List<Any>> = buildMap<String, List<Any>> {
    put(LABELS, data.map { it.label })

    val groups = data.map { it.label }.group()
    put(GROUPS, groups)

    put(X, data.map { it.x })
    put(Y, data.map { it.y })
}

val plot = letsPlot(
    plotData
) + geomLine(showLegend = true) {
    x = X
    y = Y
    group = GROUPS
    color = asDiscrete(LABELS, order = 1)
} + ggtitle(
    title
) + ylab(
    yAxisLabel
) + scaleXDateTime() + scaleColorManual(
    values = labelColors.map { it.color },
    labels = labelColors.map { it.label }
)
alshan commented 2 years ago

Hi @ccjernigan

Try to use breaks instead of labels:

scaleColorManual(
    values = labelColors.map { it.color },
    breaks = labelColors.map { it.label }
)

Maybe remove asDiscrete for beginnings. If scaleColorManual works as expected then try with asDiscrete (i.e. if you would like to have labels sorted).

ccjernigan commented 2 years ago

Thanks for the suggestion. My labels are currently strings, so I tried doing a mapping of string to ints like this:

            val labelMap = buildMap<String, Int> {
                var largestLabel = 1
                labelColors.map {
                    getOrPut(it.label) {
                        val prevLargestLabel = largestLabel
                        largestLabel++

                        prevLargestLabel
                    }
                }
            }

                plotOptions + scaleColorManual(
                    values = labelColors.map { it.color },
                    breaks = labelColors.map { labelMap[it.label]!! }
                )

It seems to help work around the issue of the wrong label being displayed in the legend. However, I think I end up losing the ability to control the specific color for a given label and the legend displays numbers instead of the semantic labels which is really helpful to understand the data. Was there something I missed in trying this suggestion?

alshan commented 2 years ago

I'm not sure I understand, how do you label your data-points in the dataframe? Is it a numeric column or a column of strings?

ccjernigan commented 2 years ago

My labels are currently a column of strings.

alshan commented 2 years ago

In this event you will use those strings in the breaks parameter as is:

scaleColorManual(
    values = labelColors.map { it.color },
    breaks = labelColors.map { it.label }
)
ccjernigan commented 2 years ago

Thanks—I think I was confused by the documentation. The docs for the breaks param say it is a list of numbers

Anyway, I just tried this with the label as a string and it does provide a workaround to the issue I originally reported.

alshan commented 2 years ago

Oh, you are right, the doc is misleading. Thanks for lets us know.