Skip to content
This repository was archived by the owner on Jul 13, 2021. It is now read-only.

Conversation

@mkborregaard
Copy link
Member

@mkborregaard mkborregaard commented Oct 31, 2018

This allows:

using AbstractPlotting, Makie
# Palette
new_theme = Theme(
    scatter = Theme(
        color = AbstractPlotting.Palette([:green, :blue, :red]),
        markersize = 0.3
    )
)

AbstractPlotting.set_theme!(new_theme)

scene = scatter(randn(100), randn(100))
scatter!(scene, randn(100), randn(100))
scatter!(scene, randn(100), randn(100))
scatter!(scene, randn(100), randn(100))

skaermbillede 2018-10-31 kl 13 19 18

You can also use a symbol

using AbstractPlotting, Makie
new_theme2 = Theme(color = AbstractPlotting.Palette(:Dark2), linewidth = 5) #for some reason the `linewidth` here is ignored
AbstractPlotting.set_theme!(new_theme2)

scene2 = plot(1:100, cumsum(randn(100)))
plot!(scene2, 1:100, cumsum(randn(100)))
plot!(scene2, 1:100, cumsum(randn(100)))
plot!(scene2, 1:100, cumsum(randn(100)))

skaermbillede 2018-10-31 kl 13 35 13

But the below doesn't work (or at least it doesn't update the scene) and I don't know if it could with this implementation:

scene[:color] = AbstractPlotting.Palette(:Dark2)

Palette should probably also have a nice show method and be exported from Makie.
I'm also not 100% sure how this behaves when adding series that are composed by series (plots composed by plots).

@Datseris
Copy link
Contributor

Does this implement a color cycle "out of the box" or one always has to setup a theme? because if setting up a theme is always necessary this doesn't bring much more difference than simply setting the colors of the plots.

@mkborregaard
Copy link
Member Author

That would be as simple as making a Palette the default color I guess. I think it should be, possibly Palette(:Dark2).

@Datseris
Copy link
Contributor

I know this may not sound very nice, but the default matplotlib cycle is not only excellent but also the result of serious research. Perhaps you want to consider it as the default for abstract plotting as well.

@mkborregaard
Copy link
Member Author

Well it sounds nice enough, but exactly the same is true for :Dark2 and the other colorbrewer color themes (see http://www.personal.psu.edu/cab38/ColorBrewer/ColorBrewer_updates.html , and in fact the colorbrewer themes are all part of the standard matplotlib color distribution), as well as e.g. wong which most of use for Plots (which has improved distinction properties for color-visually impaired, publication here: https://www.nature.com/articles/nmeth.1618 ).

The use of visually optimal color themes is in fact an important research field, and I agree we should only follow best principle here. Indeed, PlotUtils (depended on by Plots and Makie) only offers color themes that are backed by recent scientific publications to have ideal perceptual properties (e.g. https://arxiv.org/abs/1509.03700 and http://tos.org/oceanography/assets/docs/29-3_thyng.pdf ).

But on the other hand the common notion among matplotlib users that the inbuilt color cycle, and it's inbuilt viridis color scheme, is somehow objectively superior to other good color themes has no scientific basis. While you can say that some color maps are objectively good, there is no "best" theme. Just like the widespread belief that Python is objectively superior to all other languages and you can just use numba has no solid basis.

What I do think we should offer is a matplotlib, a gr and a plotly theme, etc., and a mechanism for defining persistent theming in the startup.jl file. That would allow any user to seamlessly and invisibly opt into exactly the color scheme he prefers.

@mkborregaard
Copy link
Member Author

Or would it make sense to implement a more general Cycler type for attributes and just have the Palette be a specific parametric version of that?

@Datseris
Copy link
Contributor

I am very impressed by your knowledge on the topic and I did not know that Dark2 had the same properties. I guess I was mislead by the very tiny linewidth in the example. Should I have known this, I wouldn't have commented the way I did.

Although, I was indeed aware that many other color maps / cycles are just as good as matplotlib's one. This only shows that mtl's color circle is not objectively superior to everything, but it is one of the "good" ones. Since you have established that Dark2 is also one of them, I couldn't care less which one is the final choice.

What I do think we should offer is a matplotlib, a gr and a plotly theme, etc., and a mechanism for defining persistent theming in the startup.jl file. That would allow any user to seamlessly and invisibly opt into exactly the color scheme he prefers.

sounds nice, but in my eyes only has secondary priority; as long as an "objectively good" default theme is established, then having extra "mpl"/gr themes is nice but not crucial. Therefore I do not think the word "should" is very suited.

@mkborregaard
Copy link
Member Author

FWIW, matplotlib just replaced their default color cycle with Vega's https://matplotlib.org/users/dflt_style_changes.html#colors-in-default-property-cycle

@Datseris
Copy link
Contributor

Yeah when I am referring to mpls cycle, I am referring to the "new" one of version 2.0. But to the best of my knowledge this isn't very "new", I think this version was out since a year ago, if not more?

@mkborregaard
Copy link
Member Author

mkborregaard commented Oct 31, 2018

Oh, but there's a brand new matplotlib 3.0 just now, that's the change I'm talking about. The new major version is primarily an update of the look, styles and colors - I guess I'n not the only one who considers theming of primary importance :-)

EDIT: oh @Datseris I was wrong on the matplotlib version - that was indeed at the 2.0 shift. Sorry!

@mkborregaard
Copy link
Member Author

mkborregaard commented Oct 31, 2018

But just to restate the open questions I have:

  1. Should I make a show method that displays the colors?
  2. Is it an issue that changing the color cycle object post hoc does not update colors in the scene? Can we make it do that?
  3. Will this work correctly for series/plots that are composed by multiple series/plots?
  4. Do we need to implement this in a general Cycler framework (I tend to think not, but I'd like to implement this for linestyle and marker as well for black/white themes)?
  5. (Why does linewidth not work in the theme example?).

@mkborregaard
Copy link
Member Author

And one last question - what is the testing policy for Makie? Is there some kind of visual comparison tests of Plots? When adding new functionality such as this, do you want a new plot example or what is preferable?

@SimonDanisch
Copy link
Member

anything in the example databank will get tested and can be referenced in the documentation.
You just need to add a cell e.g. here: https://github.com/JuliaPlots/Makie.jl/blob/master/examples/examples2d.jl
and it should become part of the tests!
You can then reference it in the documentation like this:
https://github.com/JuliaPlots/Makie.jl/blob/master/docs/src/basic-tutorials.md#line-plot

@SimonDanisch
Copy link
Member

If you add new examples, it will first ignore the new test (but run it), untill we make a new recording of the reference images!

@SimonDanisch SimonDanisch merged commit 29efa67 into master Nov 3, 2018
@mkborregaard mkborregaard deleted the mkb/palette branch November 4, 2018 21:04
@SimonDanisch
Copy link
Member

Ah, I never realized you didn't add this as the default color :D
Should we just do that?

@piever
Copy link
Member

piever commented Nov 10, 2018

Can Palette be an AbstractArray and implement getindex and size? Otherwise it'd break the grouping system.

@mkborregaard
Copy link
Member Author

mkborregaard commented Nov 11, 2018

@SimonDanisch yes 👍
@piever I don't see why not? Do you mean length not size? I guess it would just forward those to .color? And for getindex do rem by the length first, forward the result to getindex on .color (and update .i to that value?)?

@piever
Copy link
Member

piever commented Nov 11, 2018

I'm actually not sure! I'll try a few things to see what is the least invasive change that can be done here to make it compatible with the grouping mechanism,

@mkborregaard
Copy link
Member Author

Would you like me to push a PR with the implementation I outlined above? Or do you want to wait to see what is most appropriate?

@piever
Copy link
Member

piever commented Nov 11, 2018

May be better to wait a little bit as I'm still finalizing the grouping implementation, so am not 100% sure what I'd need for Palette to work

@piever
Copy link
Member

piever commented Nov 12, 2018

Actually I'll probably do it as I need a general Palette type for other attributes (passing say a vector to the theme doesn't work), so I'll put a PR soon and the AbstractVector part can be part of it.

For me to understand, what is the intended behaviour of this wrt grouping? Say I have a palette Palette([:red, :blue, :green, :purple, :cyan]) and I plot two traces (so, :red and :blue). If I plot on top a grouped thing (in 2 groups), should it be :green and :purple? And if I plot again, should it be :cyan?

@mkborregaard
Copy link
Member Author

Yes, I think so. Every new trace/Plot/series/we_really_need_a_name_for_this gets a new color, and grouping creates that color IMHO.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants