gonum / plot

A repository for plotting and visualizing data
BSD 3-Clause "New" or "Revised" License
2.72k stars 202 forks source link

plotter: fix handling of x/y-offsets in Labels glyph boxes #711

Closed sbinet closed 3 years ago

sbinet commented 3 years ago

This CL fixes the handling of the X/Y-offsets of plotter.Labels: they were previously ignored in the glyph box computation. This CL also adds a test that exercizes offsetted labels and their glyph box.

Fixes #710.

Please take a look.

sbinet commented 3 years ago

this is a spinoff of #708 and #710. (I'll rebase #708 off that one once/when/if it's merged)

codecov-commenter commented 3 years ago

Codecov Report

Merging #711 (3933092) into master (bd0e370) will decrease coverage by 0.07%. The diff coverage is 56.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #711      +/-   ##
==========================================
- Coverage   71.91%   71.84%   -0.08%     
==========================================
  Files          56       58       +2     
  Lines        4957     5171     +214     
==========================================
+ Hits         3565     3715     +150     
- Misses       1206     1261      +55     
- Partials      186      195       +9     
Impacted Files Coverage Δ
plotter/quartile.go 70.09% <0.00%> (ø)
vg/geom.go 62.50% <0.00%> (-14.43%) :arrow_down:
plotter/boxplot.go 85.86% <100.00%> (ø)
plotter/labels.go 80.00% <100.00%> (-0.65%) :arrow_down:
vg/vggio/context.go 100.00% <0.00%> (ø)
vg/vggio/vggio.go 67.37% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update bd0e370...3933092. Read the comment docs.

sbinet commented 3 years ago

note that we're still not out of the woods: the Aq and Bg labels in red should align correctly on the (resp.) blue and green horizontal lines (they should be the baselines.)

sbinet commented 3 years ago

PTAL

sbinet commented 3 years ago

sorry, fixed loffp and not the other.

PTAL. (rebased+merged preemptively)