Port plots to Tucan

[pnezis]

In the CHANGELOG you mention future use of Tucan, so I ported some plots 😄 .

This is still early draft, but I wanted some feedback before continuing.

Comments

• Only plots in linear_regression are ported for now, if you are ok with this change I will port the rest as well
• I tried to match exactly the existing plots using Tucan API. Notice that we can simplify most plots by removing all set_x/y_domain calls since by default Tucan scatter plots set scale: [zero: false]
• Tucan is added as a git dependency since some unreleased changes were required. I will create a release and update it in the next days.
• There are some #TODOs for some minor enhancements that should be done before this is considered ready

Looking forward for any feedback.

[josevalim]

This all looks good to me. I will ask just one favor though: instead of updating the whole document, please update only the charts and their output if possible. Only because otherwise the diffs are massive. :) thank you!

[pnezis]

@josevalim done but the diff is still massive due to the included outputs. Should I just change the plotting code (and include the outputs later once this is reviewed)?

[pnezis]

This is out of topic but do you consider it a good practice to have livebook outputs version controlled? Outputs are needed only for the docs, so we could have something like this in mix.exs:

aliases: [
   docs: [
      "livebook.persist_outputs notebooks/*.livemd",
      "docs"
   ]

this way:

• outputs are not version controlled but only the actual code/markdown, you also gain a few MBs of space in cases like this where outputs include big datasets
• docs are always up to date
• if something changes or breaks mix docs will break as well
• reviews are easier :)

you only lose on the mix docs execution time.

[josevalim]

Yeah, this is tricky. The issue is that those notebooks may also be expensive and take a while to run, no?

[pnezis]

Correct this is the tradeoff. But mix docs is usually executed very rarely, and for released docs it is more important to have correct and up to date notebooks than the mix docs execution time. You could also add this as a step to the CI.

Alternatively you can just add to the docs the code without the output, and let the user run it.

[josevalim]

Yeah. Definitely food for thought but nothing we can do now anyway. So as long as you only change the VegaLite output, we are fine. Or, if the graph visualization is literally the same (only different order of fields), you can just update the code. Then once we need to re-run the whole guide for another reason, it gets up to date.

[pnezis]

Unfortunately the visualization is not identical, the visual output though is similar. So we have to live with those massive diffs :-).

Should I also port the plots of the remaining notebooks?

[josevalim]

Yes, please port!

[msluszniak]

Is there a particular reason that you use Vl.new instead of Tucan.new?

[pnezis]

Is there a particular reason that you use Vl.new instead of Tucan.new?

Tucan.new expects some "data" as input, I could add a Tucan.new/0 wrapper, but I don't think it makes much sense, wdyt? It also shows the interoperability between the two.

[msluszniak]

This is actually a very good reason. No, the wrapper wouldn't be necessary ;)

[josevalim]

Actually, for guide purposes I think the wrapper is nice. We don't have to alias both Vl and Tucan. We just use Tucan and never mention Vl at all? Would that be feasible?

[msluszniak]

Actually, for guide purposes I think the wrapper is nice. We don't have to alias both Vl and Tucan. We just use Tucan and never mention Vl at all? Would that be feasible?

That was the reason why I asked. But for now, there is also one place where the Vl is non-trivially used. However, as you mentioned it would be comfortable to use only Tucan :)

[josevalim]

If it is just an image, should we use Kino.Image instead to render it? We can also render it using markdown via Kino.Markdown. :)

[msluszniak]

Yeah, Kino.Image will work great, I forget about it when I originally wrote this

[pnezis]

If it is just an image, should we use Kino.Image instead to render it? We can also render it using markdown via Kino.Markdown. :)

Actually it's not just an image, the image is used as the background of a scatter plot. Not sure how Kino.Image can be used for this.

Not sure though why an image is used here and not a choropleth.

[pnezis]

Actually, for guide purposes I think the wrapper is nice. We don't have to alias both Vl and Tucan. We just use Tucan and never mention Vl at all? Would that be feasible?

Will add it, but Vl is still needed for some edge cases like the above example.

[msluszniak]

If it is just an image, should we use Kino.Image instead to render it? We can also render it using markdown via Kino.Markdown. :)

Actually it's not just an image, the image is used as the background of a scatter plot. Not sure how Kino.Image can be used for this.

Not sure though why an image is used here and not a choropleth.

Sorry, you're right. I thought that sth like this would work:

{:ok, binary} = Req.get("image.png")
map = Kino.Image.new(binary.body, :png)
...
Tucan.layers([map, ...])

[pnezis]

What I could do here, would be to add a Tucan.background/3 helper that prepends a layer with an image as background, it's a bit tricky though because you need to pass the same width/height as the plot's dimensions which can change in a later step.

Will try something and let you know.

[pnezis]

@josevalim @msluszniak Ported all the plots except the interactive one in the nearest neighbors notebook since Tucan does not have an API yet for interactive vega lite charts.

• I pushed only the code changes and not the outputs so you can review.
• Once this is approved I will release a new Tucan version and update the notebooks accordingly, including the plots outputs

[josevalim]

LGTM! @msluszniak makes the final decision. :)

[msluszniak]

Good to go ;)

[pnezis]

Done :)

[josevalim]

💚 💙 💜 💛 ❤️