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

RFC Switch to Colorschemes #87

Merged
merged 14 commits into from
Apr 24, 2020
Merged

RFC Switch to Colorschemes #87

merged 14 commits into from
Apr 24, 2020

Conversation

daschw
Copy link
Member

@daschw daschw commented Apr 12, 2020

Part 1

This is an attempt to switch from PlotUtils color libraries to ColorSchemes.
We define two types, ColorGradient and ColorPalette, both wrapping a ColorScheme. and differing in the methods for getindex(c, z::AbstractFloat). Gradients interpolate linearly, while palettes choose the nearest color.
The changes here are breaking, so we need a new minor release:

  • We no longer export clibrary, clibraries and cgradients. They are deleted.
  • We additionally export
    • palette (a convenience function to construct palettes from symbols pointing to colorschemes or former Plots gradients)
    • ColorPalette
    • color_list (get the vector of colors defining gradients, palettes or colorschemes)
  • cgrad(:<grad>r) is no longer working for getting the reverse gradient of cgrad(:<grad>). Now we use cgrad(:<grad>, rev = true)
  • We no longer generate a different ColorPalette depending on the background color of a Plot but leave the specified colors untouched.

Plots gradients not defined in ColorSchemes are still defined including Plots default color palette. Former gradients with a different name than the ColorSchemes equivalent are mapped to the correct colorscheme.

This requires a follow-up PR in PlotThemes, which also will be breaking. PlotThemes can no longer export palette(sym) to get the palette of a Theme. palette(sym) is now used (analog to cgrad) to get a palette from a colorscheme defined in ColorSchemes. The PlotThemes version will be renamed to theme_palette(sym)`.

Furthermore I prepared a follow-up PR for Plots. This will be kind of breaking because Plots reexports PlotUtils and PlotThemes. Typical plotting however will barely have breaking changes with two exceptions:

  • For dark backgrounds Plots will not autogenerate a different palette based on the provided one, but always use the provided palette's colorscheme.
  • plot(..., palette = grad::Symbol) will no longer spread the colors as distant as possible among the provided color scheme, but cycle through its colors. The old behavior can be achieved with plot(..., palette = cgrad(grad))

Here's the rationale behind the second change: When passed a Symbol, the palette attribute defaults to palette(sym). This way colorschemes like :Dark2 can be used with palette = :Dark2. ColorSchemes like :inferno also have to fallback to this behavior. We are only passed a symbol and we cannot know if it is a categorical colorscheme or a gradient. I users want the old behavior for gradients they can simply pass gradients to palette with e.g. cgrad(:inferno)

x = rand(3, 6)
plot(x, palette = :Dark2, lw = 2)

old
colorschemes_palette_old

new
colorschemes_palette_new

areaplot(x, palette = :blues)

old
colorschemes_palette_grad_old

new
colorschemes_palette_grad_new

areaplot(x, palette = cgrad(:blues))

both
colorschemes_palette_cgrad_new

areaplot(x, palette = palette(:blues, 6))

only new
colorschemes_palette_n_new

When a Symbol is passed to a color attribute (and it is not a Colorant), conversely, it defaults to cgrad(sym). By providing a palette explicitely we get a new feature of a categorical gradient.

function f(x, y)
    r = sqrt(x^2 + y^2)
    return cos(r) / (1 + r)
end
x = range(-3π, 3π, length = 50)

heatmap(x, x, f, c = :deep)

colorschemes_deep

heatmap(x, x, f, c = palette(:deep, 10))

colorschemes_deep_10
This also works for surface, marker_z, line_z and fill_z.

I'm unsure about two points:

  • Are these changes a deal breaker for any other dependency on PlotUtils outside the Plots ecosystem?
  • Would it be OK for this to result in a Plots 1.1 release or is this considered breaking and requires 2.0 according to SemVer?

Note also that this requires a new ColorSchemes release.

@asinghvi17
Copy link
Member

Does ColorSchemes have all the colours which were previously in PlotUtils, or do we need to add some there?

@daschw
Copy link
Member Author

daschw commented Apr 12, 2020

We had 13 ColorSchemes in PlotUtils that are not (or in a different form) in ColorSchemes. They are now stored in PlotUtils MISC_COLORSCHEMES Dict.

@daschw
Copy link
Member Author

daschw commented Apr 12, 2020

I may have to rethink ColorGradient implementation here. Maybe we should keep colors and values stored in gradients. THat would make it easier to define custom gradients like "blue to green from 0 to 0.1 and green to yellow from 0.1 to 1.

@asinghvi17
Copy link
Member

Yes, that would also need to happen for log-scaled gradients.

@mkborregaard
Copy link
Member

Hi @daschw this is great and I agree with almost all of your ideas. Comments and thoughts:

About ColorGradient, I think that was originally defined to hold both a scale and values, right? (ColorGradient has a currently unused values field, and there's a constructor that accepts a scale).

I think the palette cgrad dichotomy is a bit unclear here, in that palette doubles as a categorical cgrad. I'd think that for palettes (using the color cycler) the right behaviour would be to first take all the given values (like implemented here), and then, when running out of colors, to try to maximize distinctness. I can see there's a case for cycling as you do for areaplot(x, palette = :blues) but it's not clear for me when it would be used. Moreover, I think it's unintuitive that

areaplot(x, palette = :blues)

has a different behaviour from

areaplot(x, palette = palette(:blues))

Similar it's unintuitive that you can retrieve the old behaviour by

palette = cgrad(grad))

given that a gradient is defined as something that varies the colors sequentially, and a palette is where you'd expect them to be spaced apart.

In sum, I think there's some unclarity about the relationships between the color and palette keywords and the cgrad and palette construction functions.

I really love that there is now support for categorical gradients, though I wonder if

cgrad(:inferno, categories = 10) 

wouldn't be more intuitive to form a categorical one (it's type stable if it's always categorical when that keyword is defined)?

About your two questions at the end:

  • I guess it would require some changes in Makie too (@SimonDanisch ?)
  • I don't think we need a new major release, as the default behaviour of the Plots color scheme will not change. Almost all of Plots' direct dependents are for tests

Project.toml Outdated Show resolved Hide resolved
colors::ColorScheme
end

Base.reverse(cg::ColorGradient) = ColorGradient(reverse(cg.colors))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't this need to pass scale and alpha cg too?

@daschw
Copy link
Member Author

daschw commented Apr 14, 2020

Thanks a lot for your feedback! I think I have updated to a cleaner and more feature-rich version now.

ColorGradient is now an abstract type with new subtypes ContinuousColorGradient and CategoricalColorGradient. They can be constructed with cgrad(c, [v]; kwargs...) and cgrad(args...; kwargs..., categorical = true). Other kwargs are rev = false, scale = nothing and alpha = nothing. c can be a symbol, a vector of colors, a colorgradient, a colorscheme or a colorpalette. The optional argument v can be a vector of values splitting the interval 0..1 or a number of colors to choose from c.

function f(x, y)
    r = sqrt(x^2 + y^2)
    return cos(r) / (1 + r)
end
x = range(-2π, 2π, length = 50)

sym = :alpine
grads = [
    cgrad(sym)
    cgrad(sym, 10, categorical = true)
    cgrad(sym, [0.1, 0.15, 0.8])
    cgrad(sym, [0.1, 0.15, 0.8], categorical = true)
]

plot((heatmap(x, x, f, c = g) for g in grads)...)

gr_grads

loggrads = [
    cgrad(sym, scale = :log)
    cgrad(sym, 10, categorical = true, scale = :log)
    cgrad(sym, [0.1, 0.15, 0.8], scale = :log)
    cgrad(sym, [0.1, 0.15, 0.8], categorical = true, scale = :log)
]

plot((heatmap(x, x, f, c = g) for g in loggrads)...)

gr_loggrads

revgrads = [
    cgrad(sym, rev = true)
    cgrad(sym, 10, categorical = true, rev = true)
    cgrad(sym, [0.1, 0.15, 0.8], rev = true)
    cgrad(sym, [0.1, 0.15, 0.8], categorical = true, rev = true)
]

plot((heatmap(x, x, f, c = g) for g in revgrads)...)

gr_revgrads

logrevgrads = [
    cgrad(sym, scale = :log, rev = true)
    cgrad(sym, 10, categorical = true, scale = :log, rev = true)
    cgrad(sym, [0.1, 0.15, 0.8], scale = :log, rev = true)
    cgrad(sym, [0.1, 0.15, 0.8], categorical = true, scale = :log, rev = true)
]

plot((heatmap(x, x, f, c = g) for g in logrevgrads)...)

gr_logrevgrads

This works for GR, Plotly(JS), PyPlot and PGPPlotsX bachends.

Note that all plots from my original post still work the same way. When color palettes are passed as color attribute for a plot with gradients they are converted to CategoricalColorGradient

Some comments to @mkborregaard comments:

I think the palette cgrad dichotomy is a bit unclear here, in that palette doubles as a categorical cgrad. I'd think that for palettes (using the color cycler) the right behaviour would be to first take all the given values (like implemented here), and then, when running out of colors, to try to maximize distinctness.

You are talking about palettes being passed as gradient, right? What does running out of colors mean here for a gradient? A gradient is defined by the colors passed and interpolated linearly in between.

Moreover, I think it's unintuitive that areaplot(x, palette = :blues) has a different behaviour from areaplot(x, palette = palette(:blues)).

It does not. Note that above I used palette(:blues, 10), because the :blues colorscheme is only defined by two colors.

Similar it's unintuitive that you can retrieve the old behaviour by palette = cgrad(grad)) given that a gradient is defined as something that varies the colors sequentially, and a palette is where you'd expect them to be spaced apart.

Agreed. Actually I don't care too much about the old behaviour. I don't know If anyone uses that. I just thought this was the easiest way to keep it still available.

function Base.get(cg::CategoricalColorGradient, x::AbstractFloat, rangescale = (0.0, 1.0))
isfinite(x) || return invisible()
rangescale = get_rangescale(rangescale)
x = clamp.(x, rangescale...)
if rangescale != (0.0, 1.0)
x = ColorSchemes.remap(x, rangescale..., 0, 1)
end
return cg.colors[x == 0 ? 1 : findlast(<(x), cg.values)]
return cg.colors[x == 0 ? 1 : findlast(z -> z < x, cg.values)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just curious - why this change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the former version does not work on julia 1.0

Copy link
Member

@asinghvi17 asinghvi17 Apr 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you fix the RecipesPipeline bug on 1.0 with the splatting?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it was due to outdated precompile statements in Plots.

@mkborregaard
Copy link
Member

Thanks for answering my concerns. This is awesome.

@daschw daschw requested a review from asinghvi17 April 16, 2020 21:17
@daschw
Copy link
Member Author

daschw commented Apr 16, 2020

@asinghvi17 it would be great if you could check if these changes are fine for AbstractPlotting/Makie too.

@asinghvi17
Copy link
Member

I suspect that the introduction of a new type is already breaking...it may make sense to refactor the backends to use these types, though. @simon could we deprecate Sampler in favor of these? The alternative is to convert these to Sampler, but since they're already the same thing it might make sense to share the code.

@daschw
Copy link
Member Author

daschw commented Apr 22, 2020

I would like to go ahead with this soon. Are there any objections @asinghvi17 @SimonDanisch ?
(FYI Anshul: You mentioned the wrong Simon in your comment above)

@cormullion
Copy link

btw - ColorSchemes 3.8 is "order of magnitude faster' apparently...

@SimonDanisch
Copy link
Member

I'd say just release a breaking version and then we can figure out how to upgrade AbstractPlotting?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants