SciNim / nim-plotly

plotly wrapper for nim-lang
https://scinim.github.io/nim-plotly/
MIT License
173 stars 15 forks source link

Can't hide legend #71

Open sdmcallister opened 3 years ago

sdmcallister commented 3 years ago

showlegend doesn't seem to get set in the output.

import plotly
import random
import json
import strutils

proc doPlot*(): string =
  let
    d1 = Trace[int](`type`: PlotType.Histogram, opacity: 0.8, name:"values 1")
    d2 = Trace[int](`type`: PlotType.Histogram, opacity: 0.8, name:"values 2")

  # using ys will make a horizontal bar plot
  # using xs will make a vertical.
  d1.ys = newSeq[int](200)
  d2.ys = newSeq[int](200)

  for i, x in d1.ys:
    d1.ys[i] = rand(20)
    d2.ys[i] = rand(30)

  for i in 0..40:
    d1.ys[i] = 12

  let
    layout = Layout(
                    yaxis: Axis(title:"values"),
                    xaxis: Axis(title: "count"),
                    # barmode: BarMode.Stack,
                    showlegend: false,
                    barmode: BarMode.Overlay,
                    autosize: true)
    p = Plot[int](layout: layout, traces: @[d1, d2])

  result = """<script>
                  var config = {responsive: true}
                  Plotly.newPlot('plot0', $1, $2, config)
              </script>""" % [parseTraces(p.traces), $(%p.layout)]
brentp commented 3 years ago

hmm. looks like it only sees showlegend if legend is set: https://github.com/brentp/nim-plotly/blob/master/src/plotly/api.nim#L210

Vindaar commented 3 years ago

Yes, I vaguely remember this.

I think the reason for this was something along the lines of providing a toggle to show / hide a legend independent of whether a legend would be created. I don't remember the details anymore though.

Vindaar commented 3 years ago

Looking into this right now again, I realize what the issue was.

showLegend is a nice idea. But given that Layout uses a normal bool for that variable we run into a problem. A non initialized showLegend is false same as an explicitly set showLegend: false. That's obviously undesirable. In my opinion the solution would be to make showLegend an Option[bool]. But given that plotly promotes creating types directly with object construction syntax, we cannot hide the fact that the internal representation is an optional. And forcing users to write some(false/true) isn't all that nice either given that there is no other usage of Option in plotly. The sugar module somewhat helps here, but of course it's not really promoted and of course it's much different in syntax than actual JS plotly.

So yes, I don't know a good solution that isn't a breaking change.

Any ideas?

brentp commented 3 years ago

For this case, can't we just do something like this:

fields["showlegend"] = % (l.showlegend or l.legend != nil)
if l.legend != nil:
    fields["legend"] = % l.legend
Vindaar commented 3 years ago

Ah, I misunderstood the OP's problem. I didn't realize it's about showlegend being in the resulting JSON.

But your proposal has the problem that then showLegend cannot be used anymore to disable the legend in case it's not desired.

brentp commented 3 years ago

yeah, it's not ideal, but if a legend is specified, presumably, it's desirable to show it. one can just not specify the legend if they don't want to show it. do I miss something?