gonum/plot: New plot

[Sebastien Binet]

hi there,

now that plot.Plot has better handling of fonts (through its plot/font.Cache field), it seems it would be possible to get rid of the reliance on a rather pesky global variable together with the "comma-error" signature of the plot.New function (and, FWIW, plot.NewLegend as well).

I'd propose to replace:
```
package plot

func New() (*Plot, error) { ... }
func NewLegend() (*Legend, error) { ... }
```

with:
```
package plot

func New(hdlr text.Handler) *Plot { ... }
func NewLegend(hdlr text.Handler) *Legend { ... }
```

optionally, one could (document and) make these new functions accept a nil argument to use plot.DefaultTextHandler.
(or panic if hdlr is nil?)

furthermore, we could also modify the following functions from plot/plotter to also accept an explicit text.Handler argument to get rid of the same global variable:

```
package plotter

func NewLabels(hdlr text.Handler, d XYLabeller) (*Labels, error)
func NewBoxPlot(hdlr text.Handler, w vg.Length, loc float64, values Valuer) (*BoxPlot, error)
func NewQuartPlot(hdlr text.Handler, loc float64, values Valuer) (*QuartPlot, error)
func NewSankey(hdlr text.Handler, flows ...Flow) (*Sankey, error)
```

(here, sadly, the "comma-error" idiom must remain in place -- as the error may not come only from loading the fonts).

WDYT?

-s

[Dan Kortschak]

The plot.New function always returns a nil error now, so the error can
be removed even without changing the input parameters. The same is true
of plot.NewLegend. Though this still leaves the global, the requirement
for the user to hold the text.Handler for all new plot parts seems like
its verging on onerous.

If there were some way to have the Plot deal with that, having been
given the text.Handler, that would be nicer. This could be done by
obtaining the text.Handler from the Plot in the plotters' Plot method
(Is there a need for distinct text.Handlers? It seems not given that we
currently use a global, but is there a potential gain from having
multiple?).

Dan

[Sebastien Binet]

I can't think of a reasonnable argument for having different
text.Handler within the same plot.Plot (but users are always very
imaginatives :P), but a different text.Handler per plot.Plot (within the
same program or library), yes.

that said, we could imagine introducing a new protocol:

package text

type ResetHandler interface { // SetHandlerer? HandlerSetter ?
SetTextHandler(h Handler)
}

// ---
package plot

func (p *Plot) TextHandler() text.Handler {
return p.hdlr
}

func (p *Plot) SetTextHandler(h text.Handler) {
// set its own handler (in Title, axes, Legend)
// ...
// propagate to plotters
for i := range p.plotters {
pp := &p.plotters[i]
if pp, ok := pp.(text.ResetHandler); ok {
pp.SetTextHandler(h)
}
}
}

and have plot/plotter types (Labels, BoxPlot, Quartile and Sankey)
implement text.ResetHandler.

that way, user code could use the default plot.New function and still,
w/o too much ceremony override the default one.

p := plot.New()
p.SetTextHandler(mypkg.Handler(...))
labels, err := plotter.NewLabels(...)
p.Add(labels)
err = p.Save(...)

(p.Save (well, p.Draw) would harmonize text.Handler by looping over p.plotters and call
SetTextHandler).

another approach:
- drop the text.Handler field from text.Style
- add it as a parameter for text.Style methods that need it (Width,
Height, Rectangle)
- add an exported text.Handler field to plot.Plot
- add an exported text.Handler field to vg/draw.Canvas (that would be seeded from
plot.Plot)

that alternative approach may fit better with the overall philosophical
approach of gonum/plot.

WDYT?

-s

[Dan Kortschak]

On Fri, 2021-02-05 at 10:43 +0000, Sebastien Binet wrote:
> I can't think of a reasonnable argument for having different
> text.Handler within the same plot.Plot (but users are always very
> imaginatives :P), but a different text.Handler per plot.Plot (within
> the
> same program or library), yes.

OK, so that suggests that plot.New should take a handler argument.

This all seems a lot more complicated than just using
p.Title.TextStyle.Handler when it's needed.

> another approach:
> - drop the text.Handler field from text.Style
> - add it as a parameter for text.Style methods that need it (Width,
> Height, Rectangle)
> - add an exported text.Handler field to plot.Plot
> - add an exported text.Handler field to vg/draw.Canvas (that would be
> seeded from
> plot.Plot)
>
> that alternative approach may fit better with the overall
> philosophical
> approach of gonum/plot.
>
> WDYT?

Can we just add the handler argument to the plot.New functions and then
allow that value to be propagated via the field. Either the
plot.NewLegend also needs the param, or Plot.Draw provides it to the
Legend during Plot.Draw.

If this isn't flexible enough, we can revisit on the next release, but
it is the least invasive.

Dan

[Sebastien Binet]

Ok.

I'll prepare a PR with my original proposal (mostly already done) but also one with the alternative approach I've sketched later (which I am starting to like better as it doesn't impose on users to sprinkle their code with text.Handlers. Most of them probably don't care about those different handlers and the default suits them)

-s





-------- Original Message --------

--
You received this message because you are subscribed to the Google Groups "gonum-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to gonum-dev+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/gonum-dev/6e690b271413fcbf24d3c2b1859993831608c3e2.camel%40kortschak.io.

[Sebastien Binet]

here are the PRs:

- https://github.com/gonum/plot/pull/667

  the PR with the new plot.New and plot.NewLegend that do not use the comma-error idiom

- https://github.com/gonum/plot/pull/668

  the PR with the text.Handler field dropped from text.Style and migrated to vg/draw.Canvas.

after having implemented both options, it seems to me #667 is the less invasive, less stuttering one.

while implementing them, another 3rd option occurred to me: functional options.

that would depart from the gonum/plot API.

e.g.

func New(opts ...Option) *Plot { ... }

func NewLegend(opts ...Option) Legend { ... }

type Option func(*option)

type option struct {

    TextHandler text.Handler

}

func newOption() *option {

    return &option{TextHandler: DefaultTextHandler}

}

func WithTextHandler(hdlr text.Handler) Option {

    return func(o *option) {

        o.TextHandler = hdlr

    }

}

that said, I'm fine with not going the func-opt way but "just" with #667.

WDYT?

-s

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐

To view this discussion on the web visit https://groups.google.com/d/msgid/gonum-dev/H9_2uvUawDBSoo5LQeRie2OslD4Jno3ds0sly2I70R_-SXrweulIBOp2qEXzOOQLpJZy0rMpajO3emCz6MVnipBmt7NDoAyrsoKsPvMBaNU%3D%40sbinet.org.

[Dan Kortschak]

On Tue, 2021-02-09 at 14:01 +0000, Sebastien Binet wrote:
> here are the PRs:
>
> - https://github.com/gonum/plot/pull/667
> the PR with the new plot.New and plot.NewLegend that do not use the
> comma-error idiom
>
> - https://github.com/gonum/plot/pull/668
> the PR with the text.Handler field dropped from text.Style and
> migrated to vg/draw.Canvas.
>
> after having implemented both options, it seems to me #667 is the
> less invasive, less stuttering one.
>
> while implementing them, another 3rd option occurred to me:
> functional options.
> that would depart from the gonum/plot API.

I prefer #667, but are there reasons that you can see for #668.

We have some functional options in plot/..., but I agree, it's probably
not something to add.

Dan

[Sebastien Binet]

I am also in favor of #667.

I'll close #668.
(I initially like the idea behind #668, but the rabbit hole of re-wiring text.Handler from text.Style turned out to be too deep a hole to my taste.)

-s

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐

> ------------------------------------------------------------------------------------------------------------------------------------------------------------------------

>
> You received this message because you are subscribed to the Google Groups "gonum-dev" group.
>
> To unsubscribe from this group and stop receiving emails from it, send an email to gonum-dev+...@googlegroups.com.
>

> To view this discussion on the web visit https://groups.google.com/d/msgid/gonum-dev/b8e0705f5c5bd36271399cbd1753c22d3cca1518.camel%40kortschak.io.