toyplot
toyplot copied to clipboard
Point extents func to fit marker size + stroke
I updated the .extents() for Point Marks to return extents of its markers as msize / 2 + stroke, unless marker type is "rNxM", where it gets height and width from the marker. I'm not sure if there are other non-symmetric markers that need to be accommodated. I've had something like this implemented in toytree for a while and it seems to work well. Thought it would be good to share. I believe this addresses Issue #164
BEFORE

AFTER

CONFIRMATION
This shows the new extents seem quite accurate by suppressing the padding and margin:

Note: Since the regression test compares against reference plots, I'm not sure this commit is truly failing, since it is suggesting a change that should change many of the plots by slightly extending the domain range. But I'm not very familiar with this testing method.
@eaton-lab - I like where this is headed, but this isn't just a case where the reference images need to be updated, the code is failing at render time ... to take one example:
$ behave -i legends -n "@1.6"
Scenario Outline: Render legends -- @1.6 # features/legends.feature:16
Given a default canvas # features/steps/common.py:13 0.001s
And a set of cartesian axes # features/steps/common.py:18 0.001s
And the data is rendered as scatterplots # features/steps/legends.py:41 0.005s
And the marks are used to create a legend # features/steps/legends.py:46 0.003s
Then the figure should match the legend-scatterplots reference image # features/steps/common.py:28 0.006s
Traceback (most recent call last):
File "/Users/tshead/miniconda3/envs/toyplot/lib/python3.10/site-packages/behave/model.py", line 1329, in run
match.run(runner.context)
File "/Users/tshead/miniconda3/envs/toyplot/lib/python3.10/site-packages/behave/matchers.py", line 98, in run
self.func(context, *args, **kwargs)
File "features/steps/common.py", line 31, in step_impl
testing.assert_canvas_equal(context.canvas, reference, show_diff=show_diff)
File "/Users/tshead/src/toyplot/features/steps/testing.py", line 240, in assert_canvas_equal
sys.modules[module].render(canvas, stream)
File "/Users/tshead/src/toyplot/toyplot/html.py", line 333, in render
_render(canvas, context.copy(parent=root_xml)) # pylint: disable=no-value-for-parameter
File "/Users/tshead/miniconda3/envs/toyplot/lib/python3.10/site-packages/multipledispatch/dispatcher.py", line 278, in __call__
return func(*args, **kwargs)
File "/Users/tshead/src/toyplot/toyplot/html.py", line 805, in _render
_render(node._finalize(), context.copy(parent=svg_xml))
File "/Users/tshead/src/toyplot/toyplot/coordinates.py", line 813, in _finalize
(x, y), (left, right, top, bottom) = child.extents(["x", "y"])
ValueError: too many values to unpack (expected 2)
After poking at it a bit, it looks like toyplot.mark.Point.extents() is returning coordinates with too many dimensions (4 instead of 2) ... I think the problem is around toyplot/mark.py line 1017. Could you take a look?
Cheers, Tim
@tshead2 I see, I think this commit will fix it.
But, I think some recent change on main has broken the regression testing framework. It is failing during sudo apt-get install ffmpeg ghostscript, which is obviously not related to this commit.
Yeah, I'm testing the pull-request locally right now. The errors look more like a server offline or something.
Cheers, Tim
Hey @tshead2 ,
Let me know what you think of these regression tests. It seems the only differences are very small changes in transform tags. Is this the expected outcome of expanding the extents for markers?
Deren
Closing-and-reopening this PR to trigger re-running the tests.