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

New syntax #69

Merged
merged 16 commits into from
May 31, 2018
Merged

New syntax #69

merged 16 commits into from
May 31, 2018

Conversation

fredo-dedup
Copy link
Collaborator

This is the implementation of a new syntax for creating plots. Main principles are :

  • Removal of the |> operator to build plots incrementally and switch to a more julian style using nested functions calls. The |> operator is maintained only for the data source (i.e. dataframe |> plot(...) ) to play nicely with processes that generate iterable tables such as Query.
  • The possibility of defining plots using only Dict or Nameduples inside the plot() to provide a straightforward translation from the JSON format of Vega-Lite :
plot(
    data = @NT(url = src),
    mark = @NT(typ = :circle),
    encoding = @NT(
        x = @NT(
            field= :IMDB_Rating,
            typ = :quantitative,
            bin=@NT(maxbins=10)),
        y = @NT(
            field= :Rotten_Tomatoes_Rating,
            typ = :quantitative,
            bin=@NT(maxbins=10)),
        size = @NT(
            aggregate = :count,
            typ = :quantitative )) )

Note : the awkward @NT for defining NamedTuples should disappear with julia 0.7

  • New shortcut functions to simplify plot definitions. Currently they exist for mark and encoding arguments. The previous definition would become :
plot(
    data(url=src),
    mk.circle(),
    enc.x.quantitative(:IMDB_Rating, bin=@NT(maxbins=10)),
    enc.y.quantitative(:Rotten_Tomatoes_Rating, bin=@NT(maxbins=10)),
    enc.size.quantitative(:*, aggregate=:count) )

note that the first argument of enc. is always the field name (or :* for aggregation such as :count), or the value for encodings that take value instead of a field (e.g. enc.color.value(:green) )

  • Inline help is available by specifying the module name before : ? VegaLite.vlconfig for example. This keeps all the VegaLite help reachable without having to import 60+ functions into the Main module, at the price of a few additional keystrokes.

I have been able to make this syntax work with every examples I tested it with, including maps, query widgets, brushes some of which are not working with the current syntax. The changes proposed seem to be consistent with our previous discussion on the subject.

What do you think ?

@davidanthoff
Copy link
Member

I like a lot of things there, and I also still like a lot of things about the previous approach. I guess I'm as undecided as to what a good API would look like as ever ;) I had also worked on a bit of a redesign of the API here. It is very incomplete and while I like some things over there, there is a lot to dislike also.

Here are some random thoughts on all of that, entirely unstructured, and probably self contradictory...

  • I completely agree that we shouldn't use |> for putting together plots. I know I probably suggested it, but I agree, it is weird. So in my branch I had done the same thing as you do here: use it to pipe data in, but for nothing else.
  • I'm completely torn between a) thinking that we should mimic the JSON format as closely as possible (your PR here is way better at that then anything we had before) and at the same time thinking that these deeply nested parenthesis are really horrible when one is working on the REPL in an interactive mode...
  • I think the shortcuts could maybe combined with the altair type stuff that I have in my branch? There the idea is that you can pass a string as the first argument that can hold the field name, type and aggregation. So for example enc.x("IMDB_Rating:q") and enc.size("count():q"). I think the one thing I don't like is the quantiative etc part in the function name, that seems very verbose. Plus, I often just leave that info out in my vega-lite specs and it still seems to work, so making that a lot less central might be a good idea?
  • I'm not sure I understand what mk.circle() gets us over mark=:circle?
  • I think we should rename plot to vlplot. Avoids all sorts of name clashes with other plotting packages.

I had played with a couple of additional things in my branch:

  • I used * for composing specs (instead of the old |>.
  • I used + to create layered plots. I kind of liked that.
  • I overwrote vcat and hcat so that one could create concatenated plots like [p1 p2 etc, essentially use the matrix syntax from julia to create panels. I kind of liked that one as well.

I just (literally five minutes ago) had another thought: what about a @vlplot macro that mimics the JSON style even closer? I think something like the following would parse:

@vlplot {
  mark=:bar,
  encoding={
    x="sum(foo):q",
    y=:bar,
    color={
        field=:foo2
    }
  }
}

I'll try to play with this branch in the next couple of days, right now my reaction is purely theoretical, and one probably really needs to try it out.

I had one final thought: I kind of feel we are at a point where the underlying displaying, piping data in, saving figures, and the @vl_str macro are quite solid and stable and essentially ready to be released. At the same time I think we'll need some more time to iterate about the julia API. Do you think it might make sense to tag a new release that just doesn't have the julia API, but only the stable stuff? One can get pretty far with just the vl string macro...

In any case, very glad you are back on this! I'm still very optimistic that this can be the best plotting story for a lot of situations!

@davidanthoff davidanthoff mentioned this pull request May 17, 2018
@davidanthoff davidanthoff merged commit 34d031d into master May 31, 2018
@davidanthoff davidanthoff deleted the new_syntax branch May 31, 2018 05:28
@davidanthoff
Copy link
Member

Alright, I've merged this into master now, and then also merged #70 and then fixed all the tests, issues etc. Probably easier if we continue from having both ideas integrated on master, and then we can iterate with smaller PRs. I'll post some more thoughts on the roadmap issue how we might continue.

@fredo-dedup
Copy link
Collaborator Author

Thank you for merging ! I agree that it could be tagged in its current state.

@tkf tkf mentioned this pull request Oct 24, 2019
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.

None yet

2 participants