diff --git a/CHANGELOG.md b/CHANGELOG.md index 4266c88..f294f7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- `spacing` (default `0`): pixels of blank space between the tree panel and + the chart panel. `0` keeps the two butted together, as before. Distinct from + `shift_tree_loc`, which moves the tree within its own panel by resizing the + label strip. CLI form: `--spacing`. + ### Changed - Pin `ruff` to `>=0.16,<0.17` and state the lint rules explicitly with @@ -15,6 +22,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 rather than whatever the installed ruff defaults to. Development-only, no effect on the package. +### Fixed + +- With `connect_leader_to_label` on, strain labels were anchored exactly on the + tree panel's chart-facing edge, so the text ran up against the chart's frame + and the frame line became hard to read. The pad that the label strip already + reserved is now applied between the text and the chart, and the leader lines + stop where the text begins rather than continuing to the panel edge. The gap + is `max(3, strain_label_font_size * 0.2)` px, and the tree panel grows by that + much, so a hand-tuned `shift_tree_loc` may want the same adjustment. + ## [0.3.0] - 2026-05-19 ### Added diff --git a/src/tree_annotated_plot/_config.py b/src/tree_annotated_plot/_config.py index 659a62f..cbfbd7c 100644 --- a/src/tree_annotated_plot/_config.py +++ b/src/tree_annotated_plot/_config.py @@ -68,6 +68,15 @@ class PlotConfig: "(x-encoded).", ] = None + spacing: Annotated[ + int, + "Pixels of blank space between the tree panel and the chart panel. " + "Default 0, which butts the two together. Distinct from " + "`shift_tree_loc`, which moves the tree within its own panel by " + "resizing the label strip; `spacing` sets the gap between the panels " + "and applies whether or not `connect_leader_to_label` is on.", + ] = 0 + tree_line_width: Annotated[ float, "Stroke width (px) for the tree's branch lines. Default 2.", @@ -155,8 +164,9 @@ class PlotConfig: shift_tree_loc: Annotated[ int, "Pixels by which to shift the tree toward (positive) or away from " - "(negative) the chart. Default 0. Has no effect when " - "connect_leader_to_label is off.", + "(negative) the chart, by resizing the label strip inside the tree " + "panel. Default 0. Has no effect when connect_leader_to_label is off; " + "to set the gap between the tree and chart panels use `spacing`.", ] = 0 color_tree_by: Annotated[ diff --git a/src/tree_annotated_plot/_plot.py b/src/tree_annotated_plot/_plot.py index 01742c5..f3639e8 100644 --- a/src/tree_annotated_plot/_plot.py +++ b/src/tree_annotated_plot/_plot.py @@ -49,6 +49,7 @@ def plot( branch_length: Literal["div", "num_date"], tree_size: int = 100, tree_location: TreeLocation | None = None, + spacing: int = 0, tree_line_width: float = 2.0, tree_node_size: float = 45, leader_line_width: float = 1.0, @@ -77,6 +78,7 @@ def plot( branch_length=branch_length, tree_size=tree_size, tree_location=tree_location, + spacing=spacing, tree_line_width=tree_line_width, tree_node_size=tree_node_size, leader_line_width=leader_line_width, @@ -281,7 +283,10 @@ def _build( hoisted_config, hoisted_other = _pop_toplevel_only_attrs(new_chart) combined = _concat_for_location( - tree_chart=tree_chart, user_chart=new_chart, location=location + tree_chart=tree_chart, + user_chart=new_chart, + location=location, + spacing=config.spacing, ) _apply_combined_config(combined, hoisted_config) for k, v in hoisted_other.items(): @@ -320,6 +325,7 @@ def _concat_for_location( tree_chart: alt.TopLevelMixin, user_chart: alt.TopLevelMixin, location: TreeLocation, + spacing: int, ) -> alt.HConcatChart | alt.VConcatChart: """Concat tree and chart in the order implied by the tree's location. @@ -333,19 +339,19 @@ def _concat_for_location( user-chart marks whose color values aren't in the tree's domain. """ if location == "left": - return alt.hconcat(tree_chart, user_chart, spacing=0).resolve_scale( + return alt.hconcat(tree_chart, user_chart, spacing=spacing).resolve_scale( y="independent", color="independent" ) if location == "right": - return alt.hconcat(user_chart, tree_chart, spacing=0).resolve_scale( + return alt.hconcat(user_chart, tree_chart, spacing=spacing).resolve_scale( y="independent", color="independent" ) if location == "top": - return alt.vconcat(tree_chart, user_chart, spacing=0).resolve_scale( + return alt.vconcat(tree_chart, user_chart, spacing=spacing).resolve_scale( x="independent", color="independent" ) if location == "bottom": - return alt.vconcat(user_chart, tree_chart, spacing=0).resolve_scale( + return alt.vconcat(user_chart, tree_chart, spacing=spacing).resolve_scale( x="independent", color="independent" ) raise ValueError(f"unreachable: tree_location={location!r}") @@ -1475,6 +1481,11 @@ def _build_scale_bar_layer( _LABEL_PAD_PX_MIN = 4 _LABEL_PAD_RATIO = 0.4 # `LABEL_PAD_PX = max(MIN, font_size * RATIO)` +# Gap between the labels and the chart, kept separate from the tree-facing pad +# above because it clears the chart's frame rather than the deepest tip, and so +# wants a small fixed clearance rather than the pad's larger, faster-growing one. +_LABEL_CHART_GAP_PX_MIN = 3 +_LABEL_CHART_GAP_RATIO = 0.2 # `LABEL_CHART_GAP_PX = max(MIN, font_size * RATIO)` _LABEL_CHAR_PX_RATIO = 0.6 # rough proportional sans-serif glyph-width estimate _LABEL_HALO_RATIO = 0.6 # white-halo strokeWidth as a fraction of font_size @@ -1614,9 +1625,15 @@ def _build_tree_chart( max_name_len = max((len(n) for n in names), default=0) char_px = strain_label_font_size * _LABEL_CHAR_PX_RATIO label_pad_px = max(_LABEL_PAD_PX_MIN, strain_label_font_size * _LABEL_PAD_RATIO) + chart_gap_px = max( + _LABEL_CHART_GAP_PX_MIN, strain_label_font_size * _LABEL_CHART_GAP_RATIO + ) # Strip needs to fit the longest label plus `halo_px / 2` of halo - # extension on the leader-facing side, plus a small fixed pad. - label_pixel_width = label_pad_px + max_name_len * char_px + halo_px / 2 + # extension on the leader-facing side, a pad between the tree and the + # text, and a smaller gap between the text and the chart. + label_pixel_width = ( + label_pad_px + chart_gap_px + max_name_len * char_px + halo_px / 2 + ) strip_pixel_width = label_pixel_width - shift_tree_loc if strip_pixel_width <= 0: raise ValueError( @@ -1629,9 +1646,13 @@ def _build_tree_chart( per_pixel = branch_span / tree_size if tree_size else 0.0 extra_branch_units = strip_pixel_width * per_pixel chart_edge_branch = branch_max + extra_branch_units - tips_df = tips_df.assign(x_label=chart_edge_branch) - leader_df = tips_df[tips_df["x"] < chart_edge_branch].assign( - x2=chart_edge_branch + # Anchor the labels `chart_gap_px` *inside* the chart-facing edge rather + # than on it, so the text does not run up against the chart's frame. The + # leaders stop at the same place, where the text begins. + label_anchor_branch = chart_edge_branch - chart_gap_px * per_pixel + tips_df = tips_df.assign(x_label=label_anchor_branch) + leader_df = tips_df[tips_df["x"] < label_anchor_branch].assign( + x2=label_anchor_branch ) extended_tree_size = tree_size + strip_pixel_width else: @@ -1680,9 +1701,9 @@ def _build_tree_chart( # When connect_leader_to_label is on, the branch domain is extended # past `branch_max` to `chart_edge_branch` so the label strip has # data-units to occupy; tips at `branch_max` still sit at pixel - # `tree_size` on the panel. Each label's chart-facing edge is - # anchored at `chart_edge_branch` and aligned outward (right for - # tree on the left, left for tree on the right). + # `tree_size` on the panel. Each label's chart-facing edge sits + # `chart_gap_px` inside `chart_edge_branch`, aligned outward (right + # for tree on the left, left for tree on the right). if tree_location == "left": branch_domain = [branch_min, chart_edge_branch] text_align = "right" @@ -1783,7 +1804,8 @@ def _build_tree_chart( # strip at bottom, opposite the chart above). # tree_location="bottom" → root at bottom → branch_max at top (label # strip at top, opposite the chart below). - # Each label's chart-facing edge is anchored at `chart_edge_branch`. + # Each label's chart-facing edge sits `chart_gap_px` inside + # `chart_edge_branch`. # The text mark is rotated 270° (reads bottom-to-top), which maps # pre-rotation `align="right"` to a top anchor (text extends down) # and `align="left"` to a bottom anchor (text extends up). For tree diff --git a/tests/test_appearance_tuning.py b/tests/test_appearance_tuning.py index 7ee5444..8e5dadb 100644 --- a/tests/test_appearance_tuning.py +++ b/tests/test_appearance_tuning.py @@ -795,3 +795,30 @@ def collect_text_marks(o: Any, hits: list[dict]) -> None: collect_text_marks(tree_panel, text_marks) sizes = [m.get("fontSize") for m in text_marks if "fontSize" in m] assert 16.0 in sizes or 16 in sizes + + +def test_cli_spacing(tmp_path): + """`--spacing` reaches the saved spec's top-level concat gap.""" + tree_path, chart_path, out_path = _cli_setup( + tmp_path, _attr_auspice(), _vertical_chart(["A", "B", "C", "D"]) + ) + _run_cli( + [ + "--tree", + str(tree_path), + "--chart", + str(chart_path), + "--output", + str(out_path), + "--chart-strain-field", + "strain", + "--tree-strain-field", + "name", + "--branch-length", + "div", + "--spacing", + "9", + ] + ) + spec = json.loads(out_path.read_text()) + assert spec["spacing"] == 9 diff --git a/tests/test_cli.py b/tests/test_cli.py index 2723c5a..ca767c5 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -45,6 +45,7 @@ def test_help_lists_auto_generated_options() -> None: "--branch-length", "--tree-size", "--tree-location", + "--spacing", "--tree-line-width", "--scale-bar / --no-scale-bar", "--strict-version / --no-strict-version", diff --git a/tests/test_label_connection.py b/tests/test_label_connection.py index 91a2940..6d4a3cd 100644 --- a/tests/test_label_connection.py +++ b/tests/test_label_connection.py @@ -285,9 +285,9 @@ def test_default_leader_endpoint_stops_at_branch_max() -> None: def test_vertical_left_renders_text_with_right_align() -> None: """Vertical layout with `connect_leader_to_label=True` and default - `tree_location="left"`: tree on left, chart on right; labels flush - against the chart on the panel's right (chart-facing) edge. Text - mark uses `align="right"` and `x_label > branch_max`.""" + `tree_location="left"`: tree on left, chart on right; labels sit just + inside the panel's right (chart-facing) edge. Text mark uses + `align="right"` and `x_label > branch_max`.""" out = tree_annotated_plot.plot(_auspice(), _vertical_chart(), **_on_kw()) text = _text_layer(out) assert text is not None @@ -557,3 +557,48 @@ def test_default_off_keeps_user_axis_labels_intact() -> None: assert axis.get("labelFontWeight") == "bold" assert axis.get("labels") is not False assert axis.get("ticks") is not False + + +# ---------- label inset from the chart-facing edge ---------- + + +def test_labels_sit_inside_the_chart_facing_edge() -> None: + """Labels are anchored one pad *inside* `chart_edge_branch` rather than on + it, so the glyphs do not run up against the chart panel's frame. The panel + itself still extends to `chart_edge_branch`, which is the chart-facing end + of the branch scale's domain.""" + out = tree_annotated_plot.plot(_auspice(), _vertical_chart(), **_on_kw()) + text = _text_layer(out) + assert text is not None + # tree on the left → branch domain is [branch_min, chart_edge_branch] + chart_edge_branch = text["encoding"]["x"]["scale"]["domain"][1] + rows = _resolve_dataset(out.to_dict(), text) + assert rows + for row in rows: + assert row["x_label"] < chart_edge_branch + + +def test_labels_sit_inside_the_chart_facing_edge_horizontal() -> None: + """Same inset in horizontal layout, where the default tree_location is + "bottom" and the branch domain runs [branch_min, chart_edge_branch] on y.""" + out = tree_annotated_plot.plot(_auspice(), _horizontal_chart(), **_on_kw()) + text = _text_layer(out) + assert text is not None + chart_edge_branch = text["encoding"]["y"]["scale"]["domain"][1] + rows = _resolve_dataset(out.to_dict(), text) + assert rows + for row in rows: + assert row["x_label"] < chart_edge_branch + + +def test_leaders_stop_where_the_labels_begin() -> None: + """The leaders run to the label anchor rather than on to the panel edge, so + the dashed line meets the text instead of passing under it to the frame.""" + out = tree_annotated_plot.plot(_auspice(), _vertical_chart(), **_on_kw()) + text = _text_layer(out) + assert text is not None + anchors = {row["x_label"] for row in _resolve_dataset(out.to_dict(), text)} + assert len(anchors) == 1, f"expected one shared label anchor, got {anchors}" + (anchor,) = anchors + for row in _resolve_dataset(out.to_dict(), _leader_layer(out)): + assert row["x2"] == pytest.approx(anchor) diff --git a/tests/test_tree_location.py b/tests/test_tree_location.py index 0c4ad84..f719a9f 100644 --- a/tests/test_tree_location.py +++ b/tests/test_tree_location.py @@ -181,3 +181,55 @@ def test_right_with_x_encoded_strain_raises() -> None: tree_annotated_plot.plot( _auspice_4_tip(), _horizontal_chart(), tree_location="right", **_kw() ) + + +# ---------- spacing between the tree and chart panels ---------- + + +@pytest.mark.parametrize( + ("chart_fn", "location"), + [ + (_vertical_chart, "left"), + (_vertical_chart, "right"), + (_horizontal_chart, "top"), + (_horizontal_chart, "bottom"), + ], +) +def test_spacing_defaults_to_zero(chart_fn, location) -> None: + """The default butts the two panels together, as before `spacing` existed.""" + out = tree_annotated_plot.plot( + _auspice_4_tip(), chart_fn(), tree_location=location, **_kw() + ) + assert out.to_dict()["spacing"] == 0 + + +@pytest.mark.parametrize( + ("chart_fn", "location"), + [ + (_vertical_chart, "left"), + (_vertical_chart, "right"), + (_horizontal_chart, "top"), + (_horizontal_chart, "bottom"), + ], +) +def test_spacing_propagates_to_the_concat(chart_fn, location) -> None: + """An explicit `spacing` reaches the top-level concat spec, for every + `tree_location` — the gap is between the two panels, so it applies + uniformly to the hconcat and vconcat forms alike.""" + out = tree_annotated_plot.plot( + _auspice_4_tip(), chart_fn(), tree_location=location, spacing=7, **_kw() + ) + assert out.to_dict()["spacing"] == 7 + + +def test_spacing_does_not_disturb_panel_sizes() -> None: + """Spacing is applied along the branch axis, between panels, so neither + panel's own dimensions change and strain-row alignment is untouched.""" + out_zero = tree_annotated_plot.plot(_auspice_4_tip(), _vertical_chart(), **_kw()) + out_gap = tree_annotated_plot.plot( + _auspice_4_tip(), _vertical_chart(), spacing=12, **_kw() + ) + d_zero, d_gap = out_zero.to_dict(), out_gap.to_dict() + for i in (0, 1): + assert d_zero["hconcat"][i]["width"] == d_gap["hconcat"][i]["width"] + assert d_zero["hconcat"][i].get("height") == d_gap["hconcat"][i].get("height")