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

allow multiple series from one argument #46

Merged
merged 16 commits into from
Nov 24, 2018
Merged

allow multiple series from one argument #46

merged 16 commits into from
Nov 24, 2018

Conversation

piever
Copy link
Member

@piever piever commented Nov 17, 2018

This introduces a general mechanism to generate multiple series from convert arguments, by creating an object to represent them.

I can optionally pass functions to this multiple object to modify the theme for the specific subseries:

using Makie
f1(theme) = merge(theme, Theme(color = :red))
f2(theme) = merge(theme, Theme(color = :blue))

pl = PlotList(
    (rand(100), rand(100)),
    (rand(100), rand(100));
    transform_attributes = [f1, f2]
)

scatter(pl)

and each individual series can have their own plot type:

pl = PlotList(
    Scatter => (rand(100), rand(100)),
    Lines => (rand(100), rand(100));
    transform_attributes = [f1, f2]
)

plot(pl)

TODO:

  • Interactivity? It's a bit tricky because the number of series may change

  • Allow MultiplePlot to absorb all the themes of the individual plots, this may be tricky in general

  • Figure out whether it can be made to work nice with layout / labelling / cycling palette etc

Once this is a clean implementation I should be able to simplify (and make less buggy) the StatsMakie implementation of analysis that return multiple plots.

@piever
Copy link
Member Author

piever commented Nov 17, 2018

Update: actually implementing the correct theme here was quite easy. If we are OK with temporarily not having interactivity when plotting PlotList this one can be merged soonish and we can later add support for labelling / legend / layout / palette for PlotList in a later PR.

@piever
Copy link
Member Author

piever commented Nov 20, 2018

I've created a custom PlotSpecs type to encode plot type, arguments and transformation we wish to apply to the theme (like changing some values or adding some default).

Example:

p = PlotList(
    PlotSpecs{Scatter}((rand(10), rand(10)), theme -> merge(theme, Theme(color = :red))),
    PlotSpecs{Lines}((-pi..pi, sin), theme -> merge(theme, Theme(color = :red))),
)
plot(p)

is the translation of (hopefully I still remember the plots syntax correctly):

@series  begin
  seriestype := :scatter
  x = 1:10
  y = 1:10
  color := :red
  (x, y)
end

@series  begin
  seriestype := :line
  x = -pi..pi
  y = sin
  color := :red
  (x, y)
end

If we like it, I'd propose to consider using PlotSpecs{ptype}(args) over ptype => args in the pipeline, as it also allows us to send over to the plot info about the attributes (setting defaults, etc...).

@mkborregaard
Copy link
Member

It's a nice idea - but do you find this style clearer than the @series syntax?

@mkborregaard
Copy link
Member

I was just reminded how nice the @series syntax and columns-are-series when rewriting this plotting function to a recipe: https://github.com/Crghilardi/ForestBiometrics.jl/pull/16/files The series macro makes it very clear what is what, and the columns-are-series plays so well with the myfunc.(vec1', vec2) syntax.

@piever
Copy link
Member Author

piever commented Nov 20, 2018

It's a nice idea - but do you find this style clearer than the @series syntax?

This is more of an implementation than a syntax. A PlotList has a transform_attributes function (to modify attributes globally) and contains a list of PlotSpecs, each of them is just like a series (has positional arguments, a plot type and the ability to modify attributes). Theoretically one could (and we may want that) have the @recipe macro from recipe base convert a plots recipe into a correct overload of convert_arguments that would transform the user types into a PlotList. This is the necessary infrastructure that's needed for that to be possible.

It'd be good to discuss how to improve the syntax without introducing macros, but as long as each PlotSpecs has plot type, args and a function to alter the theme I'm happy. I suppose a well chosen largish set of constructors for PlotSpecs would already help a lot here.

The technical constrain at the moment is that PlotList need to have information about all the plot types it contains as a type parameter (that's how default_theme is implemented, see #40 for an attempt to change this)

I was just reminded how nice the @series syntax and columns-are-series

The columns-are-series syntax can be extremely convenient, that's for sure! Having this PlotList PlotSpecs objects should make it possible to implement this kind of ideas so it's a good way to start thinking about it. I'd like to figure out what's the best way to pass attributes "one per series".

IMO the main limitations of "columns-as-series" are that it requires columns to have the same length and that sometimes converting to these row vectors can be a bit verbose (as ' only works for numeric types). I would love to use Tuples for this (even though Tuple is a valid input to some attributes I guess so this would create confusion). I'm also not entirely sure what exactly can or cannot be implemented as a Makie recipe, will have to think a bit harder on this one.

@piever
Copy link
Member Author

piever commented Nov 21, 2018

Continuing the discussion from Slack, I felt the need to introduce this complex system (of passing a PlotSpecs object with a function that would be used to transform attributes as I found the following things to be important:

  1. Add "hard defaults" to a series: the value of the attribute is set no matter what the user inputs - this is possible already in AbstractPlotting

  2. Add "soft defaults" to a series: the value of the attribute is set but if the user or the theme specifies a keyword argument, it gets overwritten. This could be achieved by adding a specific DefaltsTo type, so one could pass DefaultsTo(:red) as value of color, and Makie would now that the type passed by the user wins.

Note that in 1) and 2) we'd need to figure out whether we want to distinguish if something comes from the theme or from a keyword the user passed. For example, color is always set by the theme, so I'm not sure how 2) would play with that. Maybe the theme could give DefaultsTo(:black) to signify that recipes have precedence.

  1. Same as 1) and 2) but for all series. This is the difference between transform_attributes in a PlotSpecs and transform_attributes in a PlotList: for example, when I do grouping, I want to set colorrange for all series. This could probably be done in the PlotList recipe though.

  2. I want the new value to be a lift version of the old value (I was using this in the grouping thing when I'm splitting data that is an array and color is also an array, I want the new color to be lift(t -> t[idxs], theme[:color])

  3. Allow attributes to be set as a function of other attributes. This may be a bit far fetched, but I thought it's good to have an option for this. For example I may want to set attribute linewidth as a function of attribute linestyle, but attribute linestyle is still not available on convert_arguments

Overall however it feels like all of these are best handled as a carefully chosen set of custom types that can be passed to each attribute individually. Potentially the main ones could be:

attr = Delayed(f, syms)

which would be converted into attr = lift(f, (theme[sym] for sym in syms)...) and:

DefaultsTo(value)

which would be converted to attr = get(theme, attr, value)

@mkborregaard
Copy link
Member

(FWIW I think the precedence should be user_keyword trumps recipe trumps theme)

@piever
Copy link
Member Author

piever commented Nov 23, 2018

@SimonDanisch I was planning to merge this soonish, are you happy with the current state of the PR? My idea is that PlotSpec is a reasonably simple thing that would stay stable, whereas PlotList and MultiplePlot will still be "experimental" and may need to be changed to implement the multipe series recipe properly with interactivity etc.

@SimonDanisch SimonDanisch merged commit 57621c3 into master Nov 24, 2018
@SimonDanisch SimonDanisch deleted the pv/multiple branch November 24, 2018 15:28
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.

None yet

3 participants