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

Incorrect behavior when color vectors of intermediate lengths passed #1153

Open
tlnagy opened this issue May 27, 2018 · 0 comments
Open

Incorrect behavior when color vectors of intermediate lengths passed #1153

tlnagy opened this issue May 27, 2018 · 0 comments
Labels

Comments

@tlnagy
Copy link
Member

tlnagy commented May 27, 2018

While working on #1152, I noticed some troublesome behavior related to the usage of cycle for the color aesthetic. It occurs when the length of the color vector is > 1 and < n where n is the length of the other aesthetics.

length(color) == n

using RDatasets, Gadfly
data = dataset("datasets", "HairEyeColor")
plot(data, x=:Freq, color=:Sex, Geom.density)

image

This is the correct behavior

length(color) == 1

plot(data, x=:Freq, color=[colorant"deepskyblue"], Geom.density)

image

Also correct behavior

1 < length(color) < n

plot(data, x=:Freq, color=[colorant"deepskyblue", colorant"orange"], Geom.density)

image

This looks really weird. It turns out that what is happening is happening is that it assigns the first x value to blue, the next to orange, followed by blue, etc. I don't think there is any reason that the user would predict this behavior. Here's where it happens:

Gadfly.jl/src/statistics.jl

Lines 511 to 516 in f28c02c

for (x, c) in zip(aes.x, cycle(aes.color))
if !haskey(groups, c)
groups[c] = Float64[x]
else
push!(groups[c], x)
end

Other geometries (Geom.line, etc) prohibit the color vector from having a different dimension from the other aesthetics. I think this is a safer approach, even if it gets rid of the convenience situation of length(color) == 1. Our other option is enforce that length(color) == 1 or == n for all geometries. I think we should be consistent either way.

Also, it looks like several other geometries (notably Geom.histogram and Geom.boxplot) also make this same mistake

@tlnagy tlnagy added the bug label May 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant