8000 Document and test _get_packed_offsets() by timhoffm · Pull Request #14516 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]

Skip to content

Document and test _get_packed_offsets() #14516

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 46 additions & 7 deletions lib/matplotlib/offsetbox.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"""
The OffsetBox is a simple container artist. The child artist are meant
to be drawn at a relative position to its parent. The [VH]Packer,
The `.OffsetBox` is a simple container artist. Its child artists are
meant to be drawn at a relative position to OffsetBox. The [VH]Packer,
DrawingArea and TextArea are derived from the OffsetBox.

The [VH]Packer automatically adjust the relative positions of their
Expand Down Expand Up @@ -49,12 +49,48 @@ def _get_packed_offsets(wd_list, total, sep, mode="fixed"):
*mode*. xdescent is analogous to the usual descent, but along the
x-direction. xdescent values are currently ignored.

*wd_list* : list of (width, xdescent) of boxes to be packed.
*sep* : spacing between boxes
*total* : Intended total length. None if not used.
*mode* : packing mode. 'fixed', 'expand', or 'equal'.
For simplicity of the description, the terminology used here assumes a
horizontal layout, but the function works equally for a vertical layout.

There are three packing modes:

- 'fixed': The elements are packed tight to the left with a spacing of
*sep* in between. If *total* is *None* the returned total will be the
right edge of the last box. A non-*None* total will be passed unchecked
to the output. In particular this means that right edge of the last
box may be further to the right than the returned total.

- 'expand': Distribute the boxes with equal spacing so that the left edge
of the first box is at 0, and the right edge of the last box is at
*total*. The parameter *sep* is ignored in this mode. A total of *None*
is accepted and considered equal to 1. The total is returned unchanged
(except for the conversion *None* to 1). If the total is smaller than
the sum of the widths, the laid out boxes will overlap.

- 'equal': If *total* is given, the total space is divided in N equal
ranges and each box is left-aligned within its subspace.
Otherwise (*total* is *None*), *sep* must be provided and each box is
left-aligned in its subspace of width ``(max(widths) + sep)``. The
total width is then calculated to be ``N * (max(widths) + sep)``.

Parameters
----------
wd_list : list of (float, float)
(width, xdescent) of boxes to be packed.
total : float or None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
total : float or None
total : float or *None*

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally, I don't think we style parameter types.

Intended total length. *None* if not used.
sep : float
Spacing between boxes.
mode : {'fixed', 'expand', 'equal'}
The packing mode.

Returns
-------
total : float
The total width needed to accommodate the laid out boxes.
offsets : array of float
The left offsets of the boxes.
"""

w_list, d_list = zip(*wd_list)
# d_list is currently not used.

Expand All @@ -81,6 +117,9 @@ def _get_packed_offsets(wd_list, total, sep, mode="fixed"):
elif mode == "equal":
maxh = max(w_list)
if total is None:
if sep is None:
raise ValueError("total and sep cannot both be None when "
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This raised a TypeError before in the line below (int + None).

Since the function is private, we can do the change without announcement.

"using layout mode 'equal'.")
total = (maxh + sep) * len(w_list)
else:
sep = total / len(w_list) - maxh
Expand Down
56 changes: 56 additions & 0 deletions lib/matplotlib/tests/test_offsetbox.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from collections import namedtuple
import numpy.testing as nptest
import pytest
from matplotlib.testing.decorators import image_comparison
import matplotlib.pyplot as plt
Expand Down Expand Up @@ -123,4 +125,58 @@ def test_get_packed_offsets(wd_list, total, sep, mode):
# issue tickets (at least #10476 and #10784) related to corner cases
# triggered inside this function when calling higher-level functions
# (e.g. `Axes.legend`).
# These are just some additional smoke tests. The output is untested.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this test could just be deleted as it is essentially covered by the tests below?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not exactly, e.g. I do not test negative total or sep. I agree that they'd better be tested explicitly for their output, but until then this is better than nothing.

Since it is not documented what should happen in this cases, I've left this as is (unresolved). IMHO that discussion is worth some thought and a separate PR.

_get_packed_offsets(wd_list, total, sep, mode=mode)


_Params = namedtuple('_params', 'wd_list, total, sep, expected')


@pytest.mark.parametrize('wd_list, total, sep, expected', [
_Params( # total=None
[(3, 0), (1, 0), (2, 0)], total=None, sep=1, expected=(8, [0, 4, 6])),
_Params( # total larger than required
[(3, 0), (1, 0), (2, 0)], total=10, sep=1, expected=(10, [0, 4, 6])),
_Params( # total smaller than required
[(3, 0), (1, 0), (2, 0)], total=5, sep=1, expected=(5, [0, 4, 6])),
])
def test_get_packed_offsets_fixed(wd_list, total, sep, expected):
result = _get_packed_offsets(wd_list, total, sep, mode='fixed')
assert result[0] == expected[0]
nptest.assert_allclose(result[1], expected[1])


@pytest.mark.parametrize('wd_list, total, sep, expected', [
_Params( # total=None (implicit 1)
[(.1, 0)] * 3, total=None, sep=None, expected=(1, [0, .45, .9])),
_Params( # total larger than sum of widths
[(3, 0), (1, 0), (2, 0)], total=10, sep=1, expected=(10, [0, 5, 8])),
_Params( # total smaller sum of widths: overlapping boxes
[(3, 0), (1, 0), (2, 0)], total=5, sep=1, expected=(5, [0, 2.5, 3])),
])
def test_get_packed_offsets_expand(wd_list, total, sep, expected):
result = _get_packed_offsets(wd_list, total, sep, mode='expand')
assert result[0] == expected[0]
nptest.assert_allclose(result[1], expected[1])


@pytest.mark.parametrize('wd_list, total, sep, expected', [
_Params( # total larger than required
[(3, 0), (2, 0), (1, 0)], total=6, sep=None, expected=(6, [0, 2, 4])),
_Params( # total smaller sum of widths: overlapping boxes
[(3, 0), (2, 0), (1, 0), (.5, 0)], total=2, sep=None,
expected=(2, [0, 0.5, 1, 1.5])),
_Params( # total larger than required
[(.5, 0), (1, 0), (.2, 0)], total=None, sep=1,
expected=(6, [0, 2, 4])),
# the case total=None, sep=None is tested separately below
])
def test_get_packed_offsets_equal(wd_list, total, sep, expected):
result = _get_packed_offsets(wd_list, total, sep, mode='equal')
assert result[0] == expected[0]
nptest.assert_allclose(result[1], expected[1])


def test_get_packed_offsets_equal_total_none_sep_none():
with pytest.raises(ValueError):
_get_packed_offsets([(1, 0)] * 3, total=None, sep=None, mode='equal')
0