Open charlesdaniels opened 11 months ago
it's coming from padY
:
https://github.com/gonum/plot/blob/342a5ce/plot.go#L313...L333
func padY(p *Plot, c draw.Canvas) draw.Canvas {
glyphs := p.GlyphBoxes(p)
b := bottomMost(&c, glyphs)
yAxis := verticalAxis{p.Y}
glyphs = append(glyphs, yAxis.GlyphBoxes(p)...)
t := topMost(&c, glyphs)
miny := c.Min.Y - b.Min.Y
maxy := c.Max.Y - (t.Min.Y + t.Size().Y)
by := vg.Length(b.Y)
ty := vg.Length(t.Y)
n := (by*maxy - ty*miny) / (by - ty)
m := ((by-1)*maxy - ty*miny + miny) / (by - ty)
return draw.Canvas{
Canvas: vg.Canvas(c),
Rectangle: vg.Rectangle{
Min: vg.Point{Y: n, X: c.Min.X},
Max: vg.Point{Y: m, X: c.Max.X},
},
}
}
for the dimensions you chose, by
and ty
are equal( 0.5
) (actually, it's even worse: b == t
)
so both n
and m
are +Inf
.
(the same issue may also happen in padX
)
@kortschak what would be the regularization for this singularity ?
I don't see how we can regularise this; we are being asked to fit an elephant into a matchbox, which is not something we can do. We probably should return a meaningful error here though. There could be a check in (*Plot).WriterTo
for elements being oversized for the output. Ideally this would be returned from Draw
since it would mean that users would see the errors too if they don't use WriterTo
in their code, but that method doesn't return an error, so we can't do that.
One possibility would be to add an Err
method to Plot
that Draw
can communicate through. It's not ideal, but it is backwards compatible and at least allows discovery of failures during plot drawing.
Or:
All options (except the exploding matchbox one) suffer from the fact that the error/panic will be hard to link back to the problematic plotter.
Another would be to draw only the glyph boxes in the case that there is an overflow (filled so that if it is completely overflowed it shows up). This at least gives an indication that something is wrong to look for.
I am not sure I understand what you mean
If we render obviously wrong red blocks on the plot instead of what the user is expecting, the positions of the red blocks will help them figure out which part caused the failure.
if we just return c
unmodified (in padY
) under the elephant condition, we get the attached plot.
(which clearly looks bonkers)
alternatively: we could introduce a private plot.*Plot.draw
method that returns an error and use that inside plot.*Plot.Save
:
diff --git a/plot.go b/plot.go
index be19dd7..5881928 100644
--- a/plot.go
+++ b/plot.go
@@ -5,8 +5,10 @@
package plot
import (
+ "fmt"
"image/color"
"io"
"math"
"os"
"path/filepath"
@@ -138,11 +140,18 @@ func (p *Plot) Add(ps ...Plotter) {
// Draw draws a plot to a draw.Canvas.
//
// Plotters are drawn in the order in which they were
-// added to the plot. Plotters that implement the
+// added to the plot. Plotters that implement the
// GlyphBoxer interface will have their GlyphBoxes
// taken into account when padding the plot so that
// none of their glyphs are clipped.
func (p *Plot) Draw(c draw.Canvas) {
+ err := p.draw(c)
+ if err != nil {
+ panic(err)
+ }
+}
+
+func (p *Plot) draw(c draw.Canvas) error {
if p.BackgroundColor != nil {
c.SetColor(p.BackgroundColor)
c.Fill(c.Rectangle.Path())
@@ -165,21 +174,51 @@ func (p *Plot) Draw(c draw.Canvas) {
ywidth := y.size()
xheight := x.size()
- x.draw(padX(p, draw.Crop(c, ywidth, 0, 0, 0)))
- y.draw(padY(p, draw.Crop(c, 0, 0, xheight, 0)))
+ cx, err := padX(p, draw.Crop(c, ywidth, 0, 0, 0))
+ if err != nil {
+ return err
+ }
+ x.draw(cx)
+
+ cy, err := padY(p, draw.Crop(c, 0, 0, xheight, 0))
+ if err != nil {
+ return err
+ }
+ y.draw(cy)
+
+ cc, err := padX(p, draw.Crop(c, ywidth, 0, xheight, 0))
+ if err != nil {
+ return err
+ }
+
+ dc, err := padY(p, cc)
+ if err != nil {
+ return err
+ }
- dataC := padY(p, padX(p, draw.Crop(c, ywidth, 0, xheight, 0)))
for _, data := range p.plotters {
- data.Plot(dataC, p)
+ data.Plot(dc, p)
}
- p.Legend.Draw(draw.Crop(c, ywidth, 0, xheight, 0))
+ err = p.Legend.draw(draw.Crop(c, ywidth, 0, xheight, 0))
+ if err != nil {
+ return fmt.Errorf("could not draw legend: %w", err)
+ }
+ return nil
}
@@ -503,10 +567,17 @@ func (p *Plot) NominalY(names ...string) {
// - .tif|.tiff
func (p *Plot) WriterTo(w, h vg.Length, format string) (io.WriterTo, error) {
c, err := draw.NewFormattedCanvas(w, h, format)
if err != nil {
return nil, err
}
- p.Draw(draw.New(c))
+ err = p.draw(draw.New(c))
+ if err != nil {
+ return nil, fmt.Errorf("could not draw: %w", err)
+ }
return c, nil
}
that said, there's a precedent (in plot.Legend.Draw
) to panic if we are asked to do something silly or simply not doable.
I guess I'd err on just panicking (and also introducing the plot.*Plot.draw
method).
(even if I remember wishing at some point in the past that plot.*Plot.Draw
were returning an error
. so perhaps that's another weak data point for finally adding error
to that signature ?)
I'd be OK adding an error return, but I'd like to also do a visual panic on the plot as described above. This way if you ignore the error, you will see it in the output.
Note that I recall Ethan being opposed to Draw
having an error return, but I don't recall the reasoning.
if we just return c unmodified (in padY) under the elephant condition, we get the attached plot.
Maybe this is a silly question and I'm missing something obvious, but wouldn't it be possible to simply squish the box part in the vertical direction? As I understand it, the vertical size is always fixed, no? (of course this is all transposed for a vertical box plot).
Is there reason to not just make the sizes correct in the first place?
What are you trying to do?
Generate a box plot visualization on a canvas of a specified width and height.
What did you do?
https://go.dev/play/p/EzwxtwaRRnA
Unfortunately, this times out while fetching gonum, so I cannot verify that it builds and runs correctly in the Go playground. However I have tested it locally with go 1.21.1 darwin/arm64. It reproduces for me against both the 0.13.0 and 0.14.0 tags.
What did you expect to happen?
I expected both
p1.svg
andp2.svg
to contain the generated box plots. If for some reason gonum/plot was unable to render the plot correctly with the specified dimensions, I would have expected some kind of error, rather than a nil error and an empty output.What actually happened?
p1.svg
contains the expected plot, butp2.svg
is blank, with only the axes being rendered. I will attach copies ofp1.svg
andp2.svg
as generated on my local machine.p1:
p2:
I also tried using PNG outputs rather than SVG, but the results were the same, so I do not believe this issue is SVG-specific.
What version of Go and Gonum/plot are you using?
(it is also broken on 0.14.0)
Does this issue reproduce with the current master?
I tried testing against 342a5cee2153b051d94ae813861f9436c5584de2, which shows up in go.mod as
require gonum.org/v1/plot v0.14.1-0.20230902105700-342a5cee2153
for me. The behavior was the same as what I described.I believe this reproduces against the current master branch at time of writing.