From c460b145e6b8648e36a2e1b6c7b87a786940c839 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 7 Nov 2025 20:53:22 +0000 Subject: [PATCH 01/10] Initial plan From 74d3ae49f53420fa5fc63a08eb8716df8790deb2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 7 Nov 2025 21:05:51 +0000 Subject: [PATCH 02/10] Add subplot_titles, use_existing_titles, and title parameters to make_subplots Co-authored-by: jnumainville <10480451+jnumainville@users.noreply.github.com> --- plugins/plotly-express/docs/sub-plots.md | 60 +++++++ .../deephaven/plot/express/plots/subplots.py | 159 +++++++++++++++++- .../plot/express/plots/test_make_subplots.py | 139 +++++++++++++++ 3 files changed, 357 insertions(+), 1 deletion(-) diff --git a/plugins/plotly-express/docs/sub-plots.md b/plugins/plotly-express/docs/sub-plots.md index 2a5577e02..8b90db55e 100644 --- a/plugins/plotly-express/docs/sub-plots.md +++ b/plugins/plotly-express/docs/sub-plots.md @@ -34,6 +34,66 @@ tipping_plots = dx.make_subplots( ) ``` +### Adding Subplot Titles + +You can add titles to individual subplots using the `subplot_titles` parameter. Provide a list or tuple of titles in row-major order. + +```python order=tipping_plots,lunch_tips,dinner_tips +import deephaven.plot.express as dx +tips = dx.data.tips() + +lunch_tips = tips.where("Time = `Lunch`") +dinner_tips = tips.where("Time = `Dinner`") + +# Add titles to subplots +tipping_plots = dx.make_subplots( + dx.scatter(lunch_tips, x="TotalBill", y="Tip"), + dx.scatter(dinner_tips, x="TotalBill", y="Tip"), + rows=2, + subplot_titles=["Lunch Tips", "Dinner Tips"] +) +``` + +### Using Existing Titles + +You can automatically use the titles from the original figures as subplot titles by setting `use_existing_titles=True`. + +```python order=tipping_plots,lunch_tips,dinner_tips +import deephaven.plot.express as dx +tips = dx.data.tips() + +lunch_tips = tips.where("Time = `Lunch`") +dinner_tips = tips.where("Time = `Dinner`") + +# Figures with titles +lunch_chart = dx.scatter(lunch_tips, x="TotalBill", y="Tip", title="Lunch Tips") +dinner_chart = dx.scatter(dinner_tips, x="TotalBill", y="Tip", title="Dinner Tips") + +# Use existing titles as subplot titles +tipping_plots = dx.make_subplots( + lunch_chart, dinner_chart, + rows=2, + use_existing_titles=True +) +``` + +### Adding an Overall Title + +You can add an overall title to the combined subplot figure using the `title` parameter. + +```python order=tipping_plots,tips +import deephaven.plot.express as dx +tips = dx.data.tips() + +tipping_plots = dx.make_subplots( + dx.scatter(tips, x="TotalBill", y="Tip", by="Day"), + dx.histogram(tips, x="TotalBill"), + rows=2, + subplot_titles=["Daily Patterns", "Distribution"], + title="Tipping Analysis" +) +``` + ### Share Axes Share axes between plots with the `shared_xaxes` and `shared_yaxes` parameters. diff --git a/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py b/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py index a45b5aef6..b40dad8c3 100644 --- a/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py +++ b/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py @@ -211,6 +211,91 @@ def is_grid(specs: list[SubplotSpecDict] | Grid[SubplotSpecDict]) -> bool: return list_count == len(specs) and list_count > 0 +def extract_title_from_figure(fig: Figure | DeephavenFigure) -> str | None: + """Extract the title from a figure if it exists + + Args: + fig: The figure to extract the title from + + Returns: + The title string if it exists, None otherwise + + """ + if isinstance(fig, DeephavenFigure): + plotly_fig = fig.get_plotly_fig() + if plotly_fig is None: + return None + fig = plotly_fig + + layout = fig.to_dict().get("layout", {}) + title = layout.get("title") + + if title is None: + return None + + # Title can be either a string or a dict with a 'text' key + if isinstance(title, dict): + return title.get("text") + return str(title) + + +def create_subplot_annotations( + titles: list[str], + col_starts: list[float], + col_ends: list[float], + row_starts: list[float], + row_ends: list[float], + rows: int, + cols: int, +) -> list[dict]: + """Create annotations for subplot titles + + Args: + titles: List of titles for each subplot + col_starts: List of column start positions + col_ends: List of column end positions + row_starts: List of row start positions + row_ends: List of row end positions + rows: Number of rows + cols: Number of columns + + Returns: + List of annotation dictionaries for plotly + + """ + annotations = [] + + for idx, title in enumerate(titles): + if not title: # Skip empty titles + continue + + # Calculate row and col from index (row-major order, but reversed since grid is reversed) + row = idx // cols + col = idx % cols + + # Calculate x position (center of column) + x = (col_starts[col] + col_ends[col]) / 2 + + # Calculate y position (top of row with small offset) + y = row_ends[row] + + annotation = { + "text": title, + "showarrow": False, + "xref": "paper", + "yref": "paper", + "x": x, + "y": y, + "xanchor": "center", + "yanchor": "bottom", + "font": {"size": 16} + } + + annotations.append(annotation) + + return annotations + + def atomic_make_subplots( *figs: Figure | DeephavenFigure, rows: int = 0, @@ -223,6 +308,9 @@ def atomic_make_subplots( column_widths: list[float] | None = None, row_heights: list[float] | None = None, specs: list[SubplotSpecDict] | Grid[SubplotSpecDict] | None = None, + subplot_titles: list[str] | tuple[str, ...] | None = None, + use_existing_titles: bool = False, + title: str | None = None, unsafe_update_figure: Callable = default_callback, ) -> DeephavenFigure: """Create subplots. Either figs and at least one of rows and cols or grid @@ -240,6 +328,9 @@ def atomic_make_subplots( column_widths: See make_subplots row_heights: See make_subplots specs: See make_subplots + subplot_titles: See make_subplots + use_existing_titles: See make_subplots + title: See make_subplots Returns: DeephavenFigure: The DeephavenFigure with subplots @@ -287,6 +378,56 @@ def atomic_make_subplots( col_starts, col_ends = get_domains(column_widths, horizontal_spacing) row_starts, row_ends = get_domains(row_heights, vertical_spacing) + # Handle subplot titles + final_subplot_titles: list[str] = [] + + if use_existing_titles and subplot_titles is not None: + raise ValueError("Cannot use both use_existing_titles and subplot_titles") + + if use_existing_titles: + # Extract titles from existing figures + for fig_row in grid: + for fig in fig_row: + if fig is None: + final_subplot_titles.append("") + else: + extracted_title = extract_title_from_figure(fig) + final_subplot_titles.append(extracted_title if extracted_title else "") + elif subplot_titles is not None: + # Convert to list if tuple + final_subplot_titles = list(subplot_titles) + + # Pad with empty strings if needed + total_subplots = rows * cols + if len(final_subplot_titles) < total_subplots: + final_subplot_titles.extend([""] * (total_subplots - len(final_subplot_titles))) + + # Create the custom update function to add annotations and title + def custom_update_figure(fig: Figure) -> Figure: + # Add subplot title annotations if any + if final_subplot_titles: + annotations = create_subplot_annotations( + final_subplot_titles, + col_starts, + col_ends, + row_starts, + row_ends, + rows, + cols, + ) + + # Get existing annotations if any + existing_annotations = list(fig.layout.annotations) if fig.layout.annotations else [] + fig.update_layout(annotations=existing_annotations + annotations) + + # Add overall title if provided + if title: + fig.update_layout(title=title) + + # Apply user's unsafe_update_figure if provided + result = unsafe_update_figure(fig) + return result if result is not None else fig + return atomic_layer( *[fig for fig_row in grid for fig in fig_row], specs=get_new_specs( @@ -298,7 +439,7 @@ def atomic_make_subplots( shared_xaxes, shared_yaxes, ), - unsafe_update_figure=unsafe_update_figure, + unsafe_update_figure=custom_update_figure, # remove the legend title as it is likely incorrect remove_legend_title=True, ) @@ -347,6 +488,9 @@ def make_subplots( column_widths: list[float] | None = None, row_heights: list[float] | None = None, specs: list[SubplotSpecDict] | Grid[SubplotSpecDict] | None = None, + subplot_titles: list[str] | tuple[str, ...] | None = None, + use_existing_titles: bool = False, + title: str | None = None, unsafe_update_figure: Callable = default_callback, ) -> DeephavenFigure: """Create subplots. Either figs and at least one of rows and cols or grid @@ -383,6 +527,15 @@ def make_subplots( 'b' is a float that adds bottom padding 'rowspan' is an int to make this figure span multiple rows 'colspan' is an int to make this figure span multiple columns + subplot_titles: (Default value = None) + A list or tuple of titles for each subplot in row-major order. + Empty strings ("") can be included in the list if no subplot title + is desired in that space. Cannot be used with use_existing_titles. + use_existing_titles: (Default value = False) + If True, automatically extracts and uses titles from the input figures + as subplot titles. Cannot be used with subplot_titles. + title: (Default value = None) + The overall title for the combined subplot figure. unsafe_update_figure: An update function that takes a plotly figure as an argument and optionally returns a plotly figure. If a figure is not returned, the plotly figure passed will be assumed to be the return value. @@ -410,6 +563,10 @@ def make_subplots( column_widths=column_widths, row_heights=row_heights, specs=specs, + subplot_titles=subplot_titles, + use_existing_titles=use_existing_titles, + title=title, + unsafe_update_figure=unsafe_update_figure, ) exec_ctx = make_user_exec_ctx() diff --git a/plugins/plotly-express/test/deephaven/plot/express/plots/test_make_subplots.py b/plugins/plotly-express/test/deephaven/plot/express/plots/test_make_subplots.py index b27350783..10c80a7a8 100644 --- a/plugins/plotly-express/test/deephaven/plot/express/plots/test_make_subplots.py +++ b/plugins/plotly-express/test/deephaven/plot/express/plots/test_make_subplots.py @@ -572,6 +572,145 @@ def test_make_subplots_shared_variables(self): self.assert_chart_equals(horizontal_chart_one, horizontal_chart_two) + def test_make_subplots_with_subplot_titles(self): + import src.deephaven.plot.express as dx + + chart = dx.scatter(self.source, x="X", y="Y") + charts = dx.make_subplots( + chart, chart, rows=2, subplot_titles=["Plot 1", "Plot 2"] + ).to_dict(self.exporter) + + # Check that annotations were added for subplot titles + layout = charts["plotly"]["layout"] + self.assertIn("annotations", layout) + annotations = layout["annotations"] + + # Should have 2 annotations for 2 subplot titles + self.assertEqual(len(annotations), 2) + + # Check first annotation + self.assertEqual(annotations[0]["text"], "Plot 1") + self.assertEqual(annotations[0]["xref"], "paper") + self.assertEqual(annotations[0]["yref"], "paper") + self.assertFalse(annotations[0]["showarrow"]) + + # Check second annotation + self.assertEqual(annotations[1]["text"], "Plot 2") + + def test_make_subplots_with_empty_subplot_titles(self): + import src.deephaven.plot.express as dx + + chart = dx.scatter(self.source, x="X", y="Y") + charts = dx.make_subplots( + chart, chart, chart, rows=1, cols=3, subplot_titles=["Plot 1", "", "Plot 3"] + ).to_dict(self.exporter) + + # Check that annotations were added only for non-empty titles + layout = charts["plotly"]["layout"] + self.assertIn("annotations", layout) + annotations = layout["annotations"] + + # Should have 2 annotations (empty string skipped) + self.assertEqual(len(annotations), 2) + self.assertEqual(annotations[0]["text"], "Plot 1") + self.assertEqual(annotations[1]["text"], "Plot 3") + + def test_make_subplots_with_use_existing_titles(self): + import src.deephaven.plot.express as dx + + chart1 = dx.scatter(self.source, x="X", y="Y", title="First Chart") + chart2 = dx.scatter(self.source, x="X", y="Y", title="Second Chart") + charts = dx.make_subplots( + chart1, chart2, rows=2, use_existing_titles=True + ).to_dict(self.exporter) + + # Check that annotations were added with extracted titles + layout = charts["plotly"]["layout"] + self.assertIn("annotations", layout) + annotations = layout["annotations"] + + # Should have 2 annotations + self.assertEqual(len(annotations), 2) + self.assertEqual(annotations[0]["text"], "First Chart") + self.assertEqual(annotations[1]["text"], "Second Chart") + + def test_make_subplots_with_overall_title(self): + import src.deephaven.plot.express as dx + + chart = dx.scatter(self.source, x="X", y="Y") + charts = dx.make_subplots( + chart, chart, rows=2, title="Overall Title" + ).to_dict(self.exporter) + + # Check that the overall title was added + layout = charts["plotly"]["layout"] + self.assertIn("title", layout) + self.assertEqual(layout["title"], "Overall Title") + + def test_make_subplots_with_subplot_titles_and_overall_title(self): + import src.deephaven.plot.express as dx + + chart = dx.scatter(self.source, x="X", y="Y") + charts = dx.make_subplots( + chart, chart, rows=2, + subplot_titles=["Plot 1", "Plot 2"], + title="Combined Plot" + ).to_dict(self.exporter) + + # Check that both subplot titles and overall title were added + layout = charts["plotly"]["layout"] + + # Check overall title + self.assertIn("title", layout) + self.assertEqual(layout["title"], "Combined Plot") + + # Check subplot titles + self.assertIn("annotations", layout) + annotations = layout["annotations"] + self.assertEqual(len(annotations), 2) + self.assertEqual(annotations[0]["text"], "Plot 1") + self.assertEqual(annotations[1]["text"], "Plot 2") + + def test_make_subplots_use_existing_titles_with_grid(self): + import src.deephaven.plot.express as dx + + chart1 = dx.scatter(self.source, x="X", y="Y", title="Chart A") + chart2 = dx.scatter(self.source, x="X", y="Y", title="Chart B") + chart3 = dx.scatter(self.source, x="X", y="Y", title="Chart C") + chart4 = dx.scatter(self.source, x="X", y="Y", title="Chart D") + + grid = [[chart1, chart2], [chart3, chart4]] + charts = dx.make_subplots( + grid=grid, use_existing_titles=True + ).to_dict(self.exporter) + + # Check that annotations were added with extracted titles + layout = charts["plotly"]["layout"] + self.assertIn("annotations", layout) + annotations = layout["annotations"] + + # Should have 4 annotations + self.assertEqual(len(annotations), 4) + self.assertEqual(annotations[0]["text"], "Chart A") + self.assertEqual(annotations[1]["text"], "Chart B") + self.assertEqual(annotations[2]["text"], "Chart C") + self.assertEqual(annotations[3]["text"], "Chart D") + + def test_make_subplots_conflicting_title_options(self): + import src.deephaven.plot.express as dx + + chart = dx.scatter(self.source, x="X", y="Y") + + # Should raise error when both use_existing_titles and subplot_titles are provided + with self.assertRaises(ValueError) as context: + dx.make_subplots( + chart, chart, rows=2, + subplot_titles=["Plot 1", "Plot 2"], + use_existing_titles=True + ) + + self.assertIn("Cannot use both", str(context.exception)) + if __name__ == "__main__": unittest.main() From 8adf354243d6e16f00eeaac3db24da01d067b453 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 7 Nov 2025 21:06:59 +0000 Subject: [PATCH 03/10] Apply black formatting to subplots code and tests Co-authored-by: jnumainville <10480451+jnumainville@users.noreply.github.com> --- .../deephaven/plot/express/plots/subplots.py | 48 +++++++++++-------- .../plot/express/plots/test_make_subplots.py | 46 ++++++++++-------- 2 files changed, 52 insertions(+), 42 deletions(-) diff --git a/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py b/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py index b40dad8c3..0a81b6c86 100644 --- a/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py +++ b/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py @@ -226,13 +226,13 @@ def extract_title_from_figure(fig: Figure | DeephavenFigure) -> str | None: if plotly_fig is None: return None fig = plotly_fig - + layout = fig.to_dict().get("layout", {}) title = layout.get("title") - + if title is None: return None - + # Title can be either a string or a dict with a 'text' key if isinstance(title, dict): return title.get("text") @@ -264,21 +264,21 @@ def create_subplot_annotations( """ annotations = [] - + for idx, title in enumerate(titles): if not title: # Skip empty titles continue - + # Calculate row and col from index (row-major order, but reversed since grid is reversed) row = idx // cols col = idx % cols - + # Calculate x position (center of column) x = (col_starts[col] + col_ends[col]) / 2 - + # Calculate y position (top of row with small offset) y = row_ends[row] - + annotation = { "text": title, "showarrow": False, @@ -288,11 +288,11 @@ def create_subplot_annotations( "y": y, "xanchor": "center", "yanchor": "bottom", - "font": {"size": 16} + "font": {"size": 16}, } - + annotations.append(annotation) - + return annotations @@ -380,10 +380,10 @@ def atomic_make_subplots( # Handle subplot titles final_subplot_titles: list[str] = [] - + if use_existing_titles and subplot_titles is not None: raise ValueError("Cannot use both use_existing_titles and subplot_titles") - + if use_existing_titles: # Extract titles from existing figures for fig_row in grid: @@ -392,16 +392,20 @@ def atomic_make_subplots( final_subplot_titles.append("") else: extracted_title = extract_title_from_figure(fig) - final_subplot_titles.append(extracted_title if extracted_title else "") + final_subplot_titles.append( + extracted_title if extracted_title else "" + ) elif subplot_titles is not None: # Convert to list if tuple final_subplot_titles = list(subplot_titles) - + # Pad with empty strings if needed total_subplots = rows * cols if len(final_subplot_titles) < total_subplots: - final_subplot_titles.extend([""] * (total_subplots - len(final_subplot_titles))) - + final_subplot_titles.extend( + [""] * (total_subplots - len(final_subplot_titles)) + ) + # Create the custom update function to add annotations and title def custom_update_figure(fig: Figure) -> Figure: # Add subplot title annotations if any @@ -415,15 +419,17 @@ def custom_update_figure(fig: Figure) -> Figure: rows, cols, ) - + # Get existing annotations if any - existing_annotations = list(fig.layout.annotations) if fig.layout.annotations else [] + existing_annotations = ( + list(fig.layout.annotations) if fig.layout.annotations else [] + ) fig.update_layout(annotations=existing_annotations + annotations) - + # Add overall title if provided if title: fig.update_layout(title=title) - + # Apply user's unsafe_update_figure if provided result = unsafe_update_figure(fig) return result if result is not None else fig diff --git a/plugins/plotly-express/test/deephaven/plot/express/plots/test_make_subplots.py b/plugins/plotly-express/test/deephaven/plot/express/plots/test_make_subplots.py index 10c80a7a8..37cb078dd 100644 --- a/plugins/plotly-express/test/deephaven/plot/express/plots/test_make_subplots.py +++ b/plugins/plotly-express/test/deephaven/plot/express/plots/test_make_subplots.py @@ -584,16 +584,16 @@ def test_make_subplots_with_subplot_titles(self): layout = charts["plotly"]["layout"] self.assertIn("annotations", layout) annotations = layout["annotations"] - + # Should have 2 annotations for 2 subplot titles self.assertEqual(len(annotations), 2) - + # Check first annotation self.assertEqual(annotations[0]["text"], "Plot 1") self.assertEqual(annotations[0]["xref"], "paper") self.assertEqual(annotations[0]["yref"], "paper") self.assertFalse(annotations[0]["showarrow"]) - + # Check second annotation self.assertEqual(annotations[1]["text"], "Plot 2") @@ -609,7 +609,7 @@ def test_make_subplots_with_empty_subplot_titles(self): layout = charts["plotly"]["layout"] self.assertIn("annotations", layout) annotations = layout["annotations"] - + # Should have 2 annotations (empty string skipped) self.assertEqual(len(annotations), 2) self.assertEqual(annotations[0]["text"], "Plot 1") @@ -628,7 +628,7 @@ def test_make_subplots_with_use_existing_titles(self): layout = charts["plotly"]["layout"] self.assertIn("annotations", layout) annotations = layout["annotations"] - + # Should have 2 annotations self.assertEqual(len(annotations), 2) self.assertEqual(annotations[0]["text"], "First Chart") @@ -638,9 +638,9 @@ def test_make_subplots_with_overall_title(self): import src.deephaven.plot.express as dx chart = dx.scatter(self.source, x="X", y="Y") - charts = dx.make_subplots( - chart, chart, rows=2, title="Overall Title" - ).to_dict(self.exporter) + charts = dx.make_subplots(chart, chart, rows=2, title="Overall Title").to_dict( + self.exporter + ) # Check that the overall title was added layout = charts["plotly"]["layout"] @@ -652,18 +652,20 @@ def test_make_subplots_with_subplot_titles_and_overall_title(self): chart = dx.scatter(self.source, x="X", y="Y") charts = dx.make_subplots( - chart, chart, rows=2, + chart, + chart, + rows=2, subplot_titles=["Plot 1", "Plot 2"], - title="Combined Plot" + title="Combined Plot", ).to_dict(self.exporter) # Check that both subplot titles and overall title were added layout = charts["plotly"]["layout"] - + # Check overall title self.assertIn("title", layout) self.assertEqual(layout["title"], "Combined Plot") - + # Check subplot titles self.assertIn("annotations", layout) annotations = layout["annotations"] @@ -678,17 +680,17 @@ def test_make_subplots_use_existing_titles_with_grid(self): chart2 = dx.scatter(self.source, x="X", y="Y", title="Chart B") chart3 = dx.scatter(self.source, x="X", y="Y", title="Chart C") chart4 = dx.scatter(self.source, x="X", y="Y", title="Chart D") - + grid = [[chart1, chart2], [chart3, chart4]] - charts = dx.make_subplots( - grid=grid, use_existing_titles=True - ).to_dict(self.exporter) + charts = dx.make_subplots(grid=grid, use_existing_titles=True).to_dict( + self.exporter + ) # Check that annotations were added with extracted titles layout = charts["plotly"]["layout"] self.assertIn("annotations", layout) annotations = layout["annotations"] - + # Should have 4 annotations self.assertEqual(len(annotations), 4) self.assertEqual(annotations[0]["text"], "Chart A") @@ -700,15 +702,17 @@ def test_make_subplots_conflicting_title_options(self): import src.deephaven.plot.express as dx chart = dx.scatter(self.source, x="X", y="Y") - + # Should raise error when both use_existing_titles and subplot_titles are provided with self.assertRaises(ValueError) as context: dx.make_subplots( - chart, chart, rows=2, + chart, + chart, + rows=2, subplot_titles=["Plot 1", "Plot 2"], - use_existing_titles=True + use_existing_titles=True, ) - + self.assertIn("Cannot use both", str(context.exception)) From c339107148b3de4365e0a8865edb5b7235c83b6f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 7 Nov 2025 21:08:05 +0000 Subject: [PATCH 04/10] Apply blacken-docs formatting to sub-plots.md documentation Co-authored-by: jnumainville <10480451+jnumainville@users.noreply.github.com> --- plugins/plotly-express/docs/sub-plots.md | 86 ++++++++++++++---------- 1 file changed, 52 insertions(+), 34 deletions(-) diff --git a/plugins/plotly-express/docs/sub-plots.md b/plugins/plotly-express/docs/sub-plots.md index 8b90db55e..e44596e80 100644 --- a/plugins/plotly-express/docs/sub-plots.md +++ b/plugins/plotly-express/docs/sub-plots.md @@ -10,27 +10,33 @@ Create a series of plots as subplots, all providing unique perspectives on the d ```python order=tipping_plots,tips import deephaven.plot.express as dx -tips = dx.data.tips() # import a ticking version of the Tips dataset + +tips = dx.data.tips() # import a ticking version of the Tips dataset # create 4 plots from within make_subplots tipping_plots = dx.make_subplots( - dx.scatter(tips, x="TotalBill", y="Tip", by="Sex", - title="Tip amount by total bill"), - dx.violin(tips, y="TotalBill", by="Day", - title="Total bill distribution by day"), + dx.scatter( + tips, x="TotalBill", y="Tip", by="Sex", title="Tip amount by total bill" + ), + dx.violin(tips, y="TotalBill", by="Day", title="Total bill distribution by day"), dx.pie( - tips - .count_by("Count", by=["Sex", "Smoker"]) + tips.count_by("Count", by=["Sex", "Smoker"]) .update_view("SmokerStatus = Smoker == `No` ? `non-smoker` : `smoker`") .update_view("SmokerLabel = Sex + ` ` + SmokerStatus"), - names="SmokerLabel", values="Count", - title="Total bill by sex and smoking status"), - dx.bar(tips - .view(["TotalBill", "Tip", "Day"]) - .avg_by("Day"), - x="Day", y=["TotalBill", "Tip"], - title="Average tip as a fraction of total bill"), - rows=2, cols=2, shared_xaxes=False, shared_yaxes=False + names="SmokerLabel", + values="Count", + title="Total bill by sex and smoking status", + ), + dx.bar( + tips.view(["TotalBill", "Tip", "Day"]).avg_by("Day"), + x="Day", + y=["TotalBill", "Tip"], + title="Average tip as a fraction of total bill", + ), + rows=2, + cols=2, + shared_xaxes=False, + shared_yaxes=False, ) ``` @@ -40,6 +46,7 @@ You can add titles to individual subplots using the `subplot_titles` parameter. ```python order=tipping_plots,lunch_tips,dinner_tips import deephaven.plot.express as dx + tips = dx.data.tips() lunch_tips = tips.where("Time = `Lunch`") @@ -50,7 +57,7 @@ tipping_plots = dx.make_subplots( dx.scatter(lunch_tips, x="TotalBill", y="Tip"), dx.scatter(dinner_tips, x="TotalBill", y="Tip"), rows=2, - subplot_titles=["Lunch Tips", "Dinner Tips"] + subplot_titles=["Lunch Tips", "Dinner Tips"], ) ``` @@ -60,6 +67,7 @@ You can automatically use the titles from the original figures as subplot titles ```python order=tipping_plots,lunch_tips,dinner_tips import deephaven.plot.express as dx + tips = dx.data.tips() lunch_tips = tips.where("Time = `Lunch`") @@ -71,9 +79,7 @@ dinner_chart = dx.scatter(dinner_tips, x="TotalBill", y="Tip", title="Dinner Tip # Use existing titles as subplot titles tipping_plots = dx.make_subplots( - lunch_chart, dinner_chart, - rows=2, - use_existing_titles=True + lunch_chart, dinner_chart, rows=2, use_existing_titles=True ) ``` @@ -83,6 +89,7 @@ You can add an overall title to the combined subplot figure using the `title` pa ```python order=tipping_plots,tips import deephaven.plot.express as dx + tips = dx.data.tips() tipping_plots = dx.make_subplots( @@ -90,7 +97,7 @@ tipping_plots = dx.make_subplots( dx.histogram(tips, x="TotalBill"), rows=2, subplot_titles=["Daily Patterns", "Distribution"], - title="Tipping Analysis" + title="Tipping Analysis", ) ``` @@ -105,7 +112,8 @@ When one axis is adjusted, all axes are adjusted to match. ```python order=tipping_plots,lunch_tips,dinner_tips import deephaven.plot.express as dx -tips = dx.data.tips() # import a ticking version of the Tips dataset + +tips = dx.data.tips() # import a ticking version of the Tips dataset # filter the tips dataset for separate lunch and dinner charts lunch_tips = tips.where("Time = `Lunch`") @@ -115,7 +123,9 @@ dinner_tips = tips.where("Time = `Dinner`") tipping_plots = dx.make_subplots( dx.scatter(lunch_tips, x="TotalBill", y="Tip", labels={"Tip": "Lunch Tips"}), dx.scatter(dinner_tips, x="TotalBill", y="Tip", labels={"Tip": "Dinner Tips"}), - rows=2, shared_yaxes="all", shared_xaxes="all" + rows=2, + shared_yaxes="all", + shared_xaxes="all", ) ``` @@ -126,7 +136,8 @@ When one y-axis is adjusted, all axes along the same row are adjusted to match. ```python order=tipping_plots,lunch_tips,dinner_tips import deephaven.plot.express as dx -tips = dx.data.tips() # import a ticking version of the Tips dataset + +tips = dx.data.tips() # import a ticking version of the Tips dataset # filter the tips dataset for separate lunch and dinner charts lunch_tips = tips.where("Time = `Lunch`") @@ -135,8 +146,9 @@ dinner_tips = tips.where("Time = `Dinner`") # create chart that shares y axes along the row tipping_plots = dx.make_subplots( dx.scatter(lunch_tips, x="TotalBill", y="Tip", labels={"Tip": "Lunch Tips"}), - dx.scatter(dinner_tips, x="TotalBill", y="Tip", labels={"Tip": "Dinner Tips"}), - cols=2, shared_yaxes=True + dx.scatter(dinner_tips, x="TotalBill", y="Tip", labels={"Tip": "Dinner Tips"}), + cols=2, + shared_yaxes=True, ) ``` @@ -144,7 +156,8 @@ To share the y axes along the same column, set `shared_yaxes` to `"columns"`. ```python order=tipping_plots,lunch_tips,dinner_tips import deephaven.plot.express as dx -tips = dx.data.tips() # import a ticking version of the Tips dataset + +tips = dx.data.tips() # import a ticking version of the Tips dataset # filter the tips dataset for separate lunch and dinner charts lunch_tips = tips.where("Time = `Lunch`") @@ -153,8 +166,9 @@ dinner_tips = tips.where("Time = `Dinner`") # create chart that shares y axes along the column tipping_plots = dx.make_subplots( dx.scatter(lunch_tips, x="TotalBill", y="Tip", labels={"Tip": "Lunch Tips"}), - dx.scatter(dinner_tips, x="TotalBill", y="Tip", labels={"Tip": "Dinner Tips"}), - rows=2, shared_yaxes="columns" + dx.scatter(dinner_tips, x="TotalBill", y="Tip", labels={"Tip": "Dinner Tips"}), + rows=2, + shared_yaxes="columns", ) ``` @@ -165,7 +179,8 @@ When one x-axis is adjusted, all axes along the same column are adjusted to matc ```python order=tipping_plots,lunch_tips,dinner_tips import deephaven.plot.express as dx -tips = dx.data.tips() # import a ticking version of the Tips dataset + +tips = dx.data.tips() # import a ticking version of the Tips dataset # filter the tips dataset for separate lunch and dinner charts lunch_tips = tips.where("Time = `Lunch`") @@ -174,8 +189,9 @@ dinner_tips = tips.where("Time = `Dinner`") # create chart that shares x axes along the column tipping_plots = dx.make_subplots( dx.scatter(lunch_tips, x="TotalBill", y="Tip", labels={"Tip": "Lunch Tips"}), - dx.scatter(dinner_tips, x="TotalBill", y="Tip", labels={"Tip": "Dinner Tips"}), - rows=2, shared_xaxes=True + dx.scatter(dinner_tips, x="TotalBill", y="Tip", labels={"Tip": "Dinner Tips"}), + rows=2, + shared_xaxes=True, ) ``` @@ -183,7 +199,8 @@ To share the x axes along the same column, set `shared_yaxes` to `"columns"`. ```python order=tipping_plots,lunch_tips,dinner_tips import deephaven.plot.express as dx -tips = dx.data.tips() # import a ticking version of the Tips dataset + +tips = dx.data.tips() # import a ticking version of the Tips dataset # filter the tips dataset for separate lunch and dinner charts lunch_tips = tips.where("Time = `Lunch`") @@ -192,8 +209,9 @@ dinner_tips = tips.where("Time = `Dinner`") # create chart that shares x axes along the row tipping_plots = dx.make_subplots( dx.scatter(lunch_tips, x="TotalBill", y="Tip", labels={"Tip": "Lunch Tips"}), - dx.scatter(dinner_tips, x="TotalBill", y="Tip", labels={"Tip": "Dinner Tips"}), - cols=2, shared_xaxes="rows" + dx.scatter(dinner_tips, x="TotalBill", y="Tip", labels={"Tip": "Dinner Tips"}), + cols=2, + shared_xaxes="rows", ) ``` From 6292cc4534e4c325f6136e7413a7e3a250e1ade5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 7 Nov 2025 21:10:46 +0000 Subject: [PATCH 05/10] Fix row calculation for subplot titles to account for reversed grid Co-authored-by: jnumainville <10480451+jnumainville@users.noreply.github.com> --- .../src/deephaven/plot/express/plots/subplots.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py b/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py index 0a81b6c86..035354354 100644 --- a/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py +++ b/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py @@ -269,9 +269,13 @@ def create_subplot_annotations( if not title: # Skip empty titles continue - # Calculate row and col from index (row-major order, but reversed since grid is reversed) - row = idx // cols + # Calculate row and col from index (row-major order in user's view) + # Since the grid is reversed internally, we need to map user's row order + # to the reversed grid. User's row 0 (top) maps to reversed grid's row (rows-1) + user_row = idx // cols col = idx % cols + # Reverse the row index to match the reversed grid + row = (rows - 1) - user_row # Calculate x position (center of column) x = (col_starts[col] + col_ends[col]) / 2 @@ -386,7 +390,9 @@ def atomic_make_subplots( if use_existing_titles: # Extract titles from existing figures - for fig_row in grid: + # Since grid is already reversed, we need to extract in reversed order + # to get titles in natural top-to-bottom order + for fig_row in reversed(grid): for fig in fig_row: if fig is None: final_subplot_titles.append("") From 287266fb99c6822a7e1ea965aa1ca168677bf1bd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 7 Nov 2025 21:13:00 +0000 Subject: [PATCH 06/10] Address code review feedback: improve None handling and add title truncation logic Co-authored-by: jnumainville <10480451+jnumainville@users.noreply.github.com> --- .../deephaven/plot/express/plots/subplots.py | 11 +++++++--- .../plot/express/plots/test_make_subplots.py | 20 +++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py b/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py index 035354354..55a6e38f8 100644 --- a/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py +++ b/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py @@ -398,16 +398,21 @@ def atomic_make_subplots( final_subplot_titles.append("") else: extracted_title = extract_title_from_figure(fig) + # Explicitly check for None to handle empty strings correctly final_subplot_titles.append( - extracted_title if extracted_title else "" + extracted_title if extracted_title is not None else "" ) elif subplot_titles is not None: # Convert to list if tuple final_subplot_titles = list(subplot_titles) - # Pad with empty strings if needed + # Validate and adjust length if needed total_subplots = rows * cols - if len(final_subplot_titles) < total_subplots: + if len(final_subplot_titles) > total_subplots: + # Truncate if too many titles provided + final_subplot_titles = final_subplot_titles[:total_subplots] + elif len(final_subplot_titles) < total_subplots: + # Pad with empty strings if too few titles provided final_subplot_titles.extend( [""] * (total_subplots - len(final_subplot_titles)) ) diff --git a/plugins/plotly-express/test/deephaven/plot/express/plots/test_make_subplots.py b/plugins/plotly-express/test/deephaven/plot/express/plots/test_make_subplots.py index 37cb078dd..e84e321a0 100644 --- a/plugins/plotly-express/test/deephaven/plot/express/plots/test_make_subplots.py +++ b/plugins/plotly-express/test/deephaven/plot/express/plots/test_make_subplots.py @@ -715,6 +715,26 @@ def test_make_subplots_conflicting_title_options(self): self.assertIn("Cannot use both", str(context.exception)) + def test_make_subplots_with_too_many_subplot_titles(self): + import src.deephaven.plot.express as dx + + chart = dx.scatter(self.source, x="X", y="Y") + # Provide more titles than subplots - should be truncated + charts = dx.make_subplots( + chart, + chart, + rows=2, + subplot_titles=["Plot 1", "Plot 2", "Plot 3", "Plot 4"], + ).to_dict(self.exporter) + + # Check that only 2 annotations were created (truncated) + layout = charts["plotly"]["layout"] + self.assertIn("annotations", layout) + annotations = layout["annotations"] + self.assertEqual(len(annotations), 2) + self.assertEqual(annotations[0]["text"], "Plot 1") + self.assertEqual(annotations[1]["text"], "Plot 2") + if __name__ == "__main__": unittest.main() From 7a5f519693d8b0c300fde94bc5fb015bcb1ffa39 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 7 Nov 2025 21:15:20 +0000 Subject: [PATCH 07/10] Refactor annotation logic: extract helper function and improve empty title check Co-authored-by: jnumainville <10480451+jnumainville@users.noreply.github.com> --- .../deephaven/plot/express/plots/subplots.py | 39 ++++++++++++++----- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py b/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py index 55a6e38f8..5e514d444 100644 --- a/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py +++ b/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py @@ -239,6 +239,31 @@ def extract_title_from_figure(fig: Figure | DeephavenFigure) -> str | None: return str(title) +def map_user_index_to_grid_position(idx: int, rows: int, cols: int) -> tuple[int, int]: + """Map user's natural index to reversed grid position + + Since the grid is reversed internally (to match plotly's bottom-to-top + coordinate system), we need to convert user's natural row-major index + (top-to-bottom, left-to-right) to the reversed grid position. + + Args: + idx: User's index in natural row-major order (0 = top-left) + rows: Number of rows + cols: Number of columns + + Returns: + Tuple of (row, col) in reversed grid coordinates + + """ + # Calculate row and col from index (row-major order in user's view) + user_row = idx // cols + col = idx % cols + # Reverse the row index to match the reversed grid + # User's row 0 (top) maps to reversed grid's row (rows-1) + row = (rows - 1) - user_row + return row, col + + def create_subplot_annotations( titles: list[str], col_starts: list[float], @@ -266,21 +291,17 @@ def create_subplot_annotations( annotations = [] for idx, title in enumerate(titles): - if not title: # Skip empty titles + # Skip empty or whitespace-only titles + if not title or (isinstance(title, str) and not title.strip()): continue - # Calculate row and col from index (row-major order in user's view) - # Since the grid is reversed internally, we need to map user's row order - # to the reversed grid. User's row 0 (top) maps to reversed grid's row (rows-1) - user_row = idx // cols - col = idx % cols - # Reverse the row index to match the reversed grid - row = (rows - 1) - user_row + # Map user's natural index to reversed grid position + row, col = map_user_index_to_grid_position(idx, rows, cols) # Calculate x position (center of column) x = (col_starts[col] + col_ends[col]) / 2 - # Calculate y position (top of row with small offset) + # Calculate y position (top of row) y = row_ends[row] annotation = { From 5e6d609e3b87c87a96c61094e98f19461da3d844 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 7 Nov 2025 21:17:04 +0000 Subject: [PATCH 08/10] Simplify title check by removing redundant isinstance Co-authored-by: jnumainville <10480451+jnumainville@users.noreply.github.com> --- .../plotly-express/src/deephaven/plot/express/plots/subplots.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py b/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py index 5e514d444..7c26718ba 100644 --- a/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py +++ b/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py @@ -292,7 +292,7 @@ def create_subplot_annotations( for idx, title in enumerate(titles): # Skip empty or whitespace-only titles - if not title or (isinstance(title, str) and not title.strip()): + if not title or not title.strip(): continue # Map user's natural index to reversed grid position From eaed59c803058b40f7afdb27495fbe016778e422 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 7 Nov 2025 21:58:11 +0000 Subject: [PATCH 09/10] Refactor subplot titles implementation based on PR feedback - Rename use_existing_titles to titles_as_subtitles for clarity - Remove default value annotations from docstrings - Extract title handling logic into get_subplot_titles helper function - Move annotation and title application from unsafe_update_figure to atomic_layer - Add subplot_annotations and overall_title parameters to atomic_layer and layer - Update documentation to use simpler language (left to right, top to bottom instead of row-major) - Update all tests to use titles_as_subtitles - Simplify extract_title_from_figure to always return string Co-authored-by: jnumainville <10480451+jnumainville@users.noreply.github.com> --- plugins/plotly-express/docs/sub-plots.md | 6 +- .../deephaven/plot/express/plots/_layer.py | 21 +++ .../deephaven/plot/express/plots/subplots.py | 157 +++++++++--------- .../plot/express/plots/test_make_subplots.py | 12 +- 4 files changed, 110 insertions(+), 86 deletions(-) diff --git a/plugins/plotly-express/docs/sub-plots.md b/plugins/plotly-express/docs/sub-plots.md index e44596e80..a8f466688 100644 --- a/plugins/plotly-express/docs/sub-plots.md +++ b/plugins/plotly-express/docs/sub-plots.md @@ -42,7 +42,7 @@ tipping_plots = dx.make_subplots( ### Adding Subplot Titles -You can add titles to individual subplots using the `subplot_titles` parameter. Provide a list or tuple of titles in row-major order. +You can add titles to individual subplots using the `subplot_titles` parameter. Provide a list or tuple of titles, ordered from left to right, top to bottom. ```python order=tipping_plots,lunch_tips,dinner_tips import deephaven.plot.express as dx @@ -63,7 +63,7 @@ tipping_plots = dx.make_subplots( ### Using Existing Titles -You can automatically use the titles from the original figures as subplot titles by setting `use_existing_titles=True`. +You can automatically use the titles from the original figures as subplot titles by setting `titles_as_subtitles=True`. ```python order=tipping_plots,lunch_tips,dinner_tips import deephaven.plot.express as dx @@ -79,7 +79,7 @@ dinner_chart = dx.scatter(dinner_tips, x="TotalBill", y="Tip", title="Dinner Tip # Use existing titles as subplot titles tipping_plots = dx.make_subplots( - lunch_chart, dinner_chart, rows=2, use_existing_titles=True + lunch_chart, dinner_chart, rows=2, titles_as_subtitles=True ) ``` diff --git a/plugins/plotly-express/src/deephaven/plot/express/plots/_layer.py b/plugins/plotly-express/src/deephaven/plot/express/plots/_layer.py index a917e0052..4531d81e8 100644 --- a/plugins/plotly-express/src/deephaven/plot/express/plots/_layer.py +++ b/plugins/plotly-express/src/deephaven/plot/express/plots/_layer.py @@ -439,6 +439,8 @@ def atomic_layer( specs: list[LayerSpecDict] | None = None, unsafe_update_figure: Callable = default_callback, remove_legend_title: bool = False, + subplot_annotations: list[dict] | None = None, + overall_title: str | None = None, ) -> DeephavenFigure: """ Layers the provided figures. This is an atomic version of layer, so the @@ -460,6 +462,10 @@ def atomic_layer( should be kept, but is necessary for other layering and subplotting as they may not use the same plot by (and similar) columns, so the legend title would be incorrect. + subplot_annotations: + List of annotation dictionaries to add to the layout for subplot titles. + overall_title: + Overall title to set for the figure. Returns: The layered chart @@ -527,6 +533,17 @@ def atomic_layer( if remove_legend_title: new_fig.update_layout(legend_title_text=None) + # Add subplot annotations if provided + if subplot_annotations: + existing_annotations = ( + list(new_fig.layout.annotations) if new_fig.layout.annotations else [] + ) + new_fig.update_layout(annotations=existing_annotations + subplot_annotations) + + # Add overall title if provided + if overall_title: + new_fig.update_layout(title=overall_title) + update_wrapper = partial(unsafe_figure_update_wrapper, unsafe_update_figure) return update_wrapper( @@ -546,6 +563,7 @@ def layer( which_layout: int | None = None, specs: list[LayerSpecDict] | None = None, unsafe_update_figure: Callable = default_callback, + overall_title: str | None = None, ) -> DeephavenFigure: """Layers the provided figures. Be default, the layouts are sequentially applied, so the layouts of later figures will override the layouts of early @@ -571,6 +589,8 @@ def layer( Used to add any custom changes to the underlying plotly figure. Note that the existing data traces should not be removed. This may lead to unexpected behavior if traces are modified in a way that break data mappings. + overall_title: + Overall title to set for the figure. Returns: The layered chart @@ -588,6 +608,7 @@ def layer( # remove the legend title as it is likely incorrect remove_legend_title=True, unsafe_update_figure=unsafe_update_figure, + overall_title=overall_title, ) exec_ctx = make_user_exec_ctx() diff --git a/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py b/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py index 7c26718ba..edebc75a5 100644 --- a/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py +++ b/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py @@ -211,31 +211,31 @@ def is_grid(specs: list[SubplotSpecDict] | Grid[SubplotSpecDict]) -> bool: return list_count == len(specs) and list_count > 0 -def extract_title_from_figure(fig: Figure | DeephavenFigure) -> str | None: +def extract_title_from_figure(fig: Figure | DeephavenFigure) -> str: """Extract the title from a figure if it exists Args: fig: The figure to extract the title from Returns: - The title string if it exists, None otherwise + The title string if it exists, empty string otherwise """ if isinstance(fig, DeephavenFigure): plotly_fig = fig.get_plotly_fig() if plotly_fig is None: - return None + return "" fig = plotly_fig layout = fig.to_dict().get("layout", {}) title = layout.get("title") if title is None: - return None + return "" # Title can be either a string or a dict with a 'text' key if isinstance(title, dict): - return title.get("text") + return title.get("text", "") return str(title) @@ -321,6 +321,49 @@ def create_subplot_annotations( return annotations +def get_subplot_titles( + grid: Grid[Figure | DeephavenFigure], + subplot_titles: list[str] | tuple[str, ...] | None, + titles_as_subtitles: bool, + rows: int, + cols: int, +) -> list[str]: + """Get the list of subplot titles based on parameters + + Args: + grid: The grid of figures (already reversed) + subplot_titles: Explicit list of titles provided by user + titles_as_subtitles: Whether to extract titles from figures + rows: Number of rows + cols: Number of columns + + Returns: + List of titles in user's natural order (top-to-bottom, left-to-right) + + """ + if titles_as_subtitles and subplot_titles is not None: + raise ValueError("Cannot use both titles_as_subtitles and subplot_titles") + + if titles_as_subtitles: + # Extract titles from figures in natural order (reverse the grid) + return [ + extract_title_from_figure(fig) if fig is not None else "" + for fig_row in reversed(grid) + for fig in fig_row + ] + elif subplot_titles is not None: + # Convert to list and adjust length + titles = list(subplot_titles) + total_subplots = rows * cols + if len(titles) > total_subplots: + return titles[:total_subplots] + elif len(titles) < total_subplots: + titles.extend([""] * (total_subplots - len(titles))) + return titles + + return [] + + def atomic_make_subplots( *figs: Figure | DeephavenFigure, rows: int = 0, @@ -334,7 +377,7 @@ def atomic_make_subplots( row_heights: list[float] | None = None, specs: list[SubplotSpecDict] | Grid[SubplotSpecDict] | None = None, subplot_titles: list[str] | tuple[str, ...] | None = None, - use_existing_titles: bool = False, + titles_as_subtitles: bool = False, title: str | None = None, unsafe_update_figure: Callable = default_callback, ) -> DeephavenFigure: @@ -354,7 +397,7 @@ def atomic_make_subplots( row_heights: See make_subplots specs: See make_subplots subplot_titles: See make_subplots - use_existing_titles: See make_subplots + titles_as_subtitles: See make_subplots title: See make_subplots Returns: @@ -403,68 +446,25 @@ def atomic_make_subplots( col_starts, col_ends = get_domains(column_widths, horizontal_spacing) row_starts, row_ends = get_domains(row_heights, vertical_spacing) - # Handle subplot titles - final_subplot_titles: list[str] = [] - - if use_existing_titles and subplot_titles is not None: - raise ValueError("Cannot use both use_existing_titles and subplot_titles") - - if use_existing_titles: - # Extract titles from existing figures - # Since grid is already reversed, we need to extract in reversed order - # to get titles in natural top-to-bottom order - for fig_row in reversed(grid): - for fig in fig_row: - if fig is None: - final_subplot_titles.append("") - else: - extracted_title = extract_title_from_figure(fig) - # Explicitly check for None to handle empty strings correctly - final_subplot_titles.append( - extracted_title if extracted_title is not None else "" - ) - elif subplot_titles is not None: - # Convert to list if tuple - final_subplot_titles = list(subplot_titles) + # Get subplot titles using the helper function + final_subplot_titles = get_subplot_titles( + grid, subplot_titles, titles_as_subtitles, rows, cols + ) - # Validate and adjust length if needed - total_subplots = rows * cols - if len(final_subplot_titles) > total_subplots: - # Truncate if too many titles provided - final_subplot_titles = final_subplot_titles[:total_subplots] - elif len(final_subplot_titles) < total_subplots: - # Pad with empty strings if too few titles provided - final_subplot_titles.extend( - [""] * (total_subplots - len(final_subplot_titles)) - ) - - # Create the custom update function to add annotations and title - def custom_update_figure(fig: Figure) -> Figure: - # Add subplot title annotations if any - if final_subplot_titles: - annotations = create_subplot_annotations( - final_subplot_titles, - col_starts, - col_ends, - row_starts, - row_ends, - rows, - cols, - ) - - # Get existing annotations if any - existing_annotations = ( - list(fig.layout.annotations) if fig.layout.annotations else [] - ) - fig.update_layout(annotations=existing_annotations + annotations) - - # Add overall title if provided - if title: - fig.update_layout(title=title) - - # Apply user's unsafe_update_figure if provided - result = unsafe_update_figure(fig) - return result if result is not None else fig + # Create subplot annotations if we have titles + subplot_annotations = None + if final_subplot_titles: + subplot_annotations = create_subplot_annotations( + final_subplot_titles, + col_starts, + col_ends, + row_starts, + row_ends, + rows, + cols, + ) + + return atomic_layer([""] * (total_subplots - len(final_subplot_titles))) return atomic_layer( *[fig for fig_row in grid for fig in fig_row], @@ -477,7 +477,9 @@ def custom_update_figure(fig: Figure) -> Figure: shared_xaxes, shared_yaxes, ), - unsafe_update_figure=custom_update_figure, + unsafe_update_figure=unsafe_update_figure, + subplot_annotations=subplot_annotations, + overall_title=title, # remove the legend title as it is likely incorrect remove_legend_title=True, ) @@ -527,7 +529,7 @@ def make_subplots( row_heights: list[float] | None = None, specs: list[SubplotSpecDict] | Grid[SubplotSpecDict] | None = None, subplot_titles: list[str] | tuple[str, ...] | None = None, - use_existing_titles: bool = False, + titles_as_subtitles: bool = False, title: str | None = None, unsafe_update_figure: Callable = default_callback, ) -> DeephavenFigure: @@ -554,7 +556,7 @@ def make_subplots( vertical_spacing: Spacing between each row. Default 0.3 / rows column_widths: The widths of each column. Should sum to 1. row_heights: The heights of each row. Should sum to 1. - specs: (Default value = None) + specs: A list or grid of dicts that contain specs. An empty dictionary represents no specs, and None represents no figure, either to leave a gap on the subplots on provide room for a figure spanning @@ -565,14 +567,15 @@ def make_subplots( 'b' is a float that adds bottom padding 'rowspan' is an int to make this figure span multiple rows 'colspan' is an int to make this figure span multiple columns - subplot_titles: (Default value = None) - A list or tuple of titles for each subplot in row-major order. + subplot_titles: + A list or tuple of titles for each subplot, provided from left to right, + top to bottom. Empty strings ("") can be included in the list if no subplot title - is desired in that space. Cannot be used with use_existing_titles. - use_existing_titles: (Default value = False) + is desired in that space. Cannot be used with titles_as_subtitles. + titles_as_subtitles: If True, automatically extracts and uses titles from the input figures as subplot titles. Cannot be used with subplot_titles. - title: (Default value = None) + title: The overall title for the combined subplot figure. unsafe_update_figure: An update function that takes a plotly figure as an argument and optionally returns a plotly figure. If a figure is not @@ -602,7 +605,7 @@ def make_subplots( row_heights=row_heights, specs=specs, subplot_titles=subplot_titles, - use_existing_titles=use_existing_titles, + titles_as_subtitles=titles_as_subtitles, title=title, unsafe_update_figure=unsafe_update_figure, ) diff --git a/plugins/plotly-express/test/deephaven/plot/express/plots/test_make_subplots.py b/plugins/plotly-express/test/deephaven/plot/express/plots/test_make_subplots.py index e84e321a0..fa9948dcf 100644 --- a/plugins/plotly-express/test/deephaven/plot/express/plots/test_make_subplots.py +++ b/plugins/plotly-express/test/deephaven/plot/express/plots/test_make_subplots.py @@ -615,13 +615,13 @@ def test_make_subplots_with_empty_subplot_titles(self): self.assertEqual(annotations[0]["text"], "Plot 1") self.assertEqual(annotations[1]["text"], "Plot 3") - def test_make_subplots_with_use_existing_titles(self): + def test_make_subplots_with_titles_as_subtitles(self): import src.deephaven.plot.express as dx chart1 = dx.scatter(self.source, x="X", y="Y", title="First Chart") chart2 = dx.scatter(self.source, x="X", y="Y", title="Second Chart") charts = dx.make_subplots( - chart1, chart2, rows=2, use_existing_titles=True + chart1, chart2, rows=2, titles_as_subtitles=True ).to_dict(self.exporter) # Check that annotations were added with extracted titles @@ -673,7 +673,7 @@ def test_make_subplots_with_subplot_titles_and_overall_title(self): self.assertEqual(annotations[0]["text"], "Plot 1") self.assertEqual(annotations[1]["text"], "Plot 2") - def test_make_subplots_use_existing_titles_with_grid(self): + def test_make_subplots_titles_as_subtitles_with_grid(self): import src.deephaven.plot.express as dx chart1 = dx.scatter(self.source, x="X", y="Y", title="Chart A") @@ -682,7 +682,7 @@ def test_make_subplots_use_existing_titles_with_grid(self): chart4 = dx.scatter(self.source, x="X", y="Y", title="Chart D") grid = [[chart1, chart2], [chart3, chart4]] - charts = dx.make_subplots(grid=grid, use_existing_titles=True).to_dict( + charts = dx.make_subplots(grid=grid, titles_as_subtitles=True).to_dict( self.exporter ) @@ -703,14 +703,14 @@ def test_make_subplots_conflicting_title_options(self): chart = dx.scatter(self.source, x="X", y="Y") - # Should raise error when both use_existing_titles and subplot_titles are provided + # Should raise error when both titles_as_subtitles and subplot_titles are provided with self.assertRaises(ValueError) as context: dx.make_subplots( chart, chart, rows=2, subplot_titles=["Plot 1", "Plot 2"], - use_existing_titles=True, + titles_as_subtitles=True, ) self.assertIn("Cannot use both", str(context.exception)) From 10b605297a5afd931cdc34fc0b2a8d3e557f6e40 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 7 Nov 2025 22:21:01 +0000 Subject: [PATCH 10/10] Address PR feedback: simplify parameter names and remove padding logic - Rename 'overall_title' parameter to 'title' in layer function - Remove unnecessary padding logic from get_subplot_titles (annotations are skipped for missing titles anyway) - Remove erroneous duplicate return statement in atomic_make_subplots Co-authored-by: jnumainville <10480451+jnumainville@users.noreply.github.com> --- .../src/deephaven/plot/express/plots/_layer.py | 6 +++--- .../src/deephaven/plot/express/plots/subplots.py | 6 +----- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/plugins/plotly-express/src/deephaven/plot/express/plots/_layer.py b/plugins/plotly-express/src/deephaven/plot/express/plots/_layer.py index 4531d81e8..41ec38771 100644 --- a/plugins/plotly-express/src/deephaven/plot/express/plots/_layer.py +++ b/plugins/plotly-express/src/deephaven/plot/express/plots/_layer.py @@ -563,7 +563,7 @@ def layer( which_layout: int | None = None, specs: list[LayerSpecDict] | None = None, unsafe_update_figure: Callable = default_callback, - overall_title: str | None = None, + title: str | None = None, ) -> DeephavenFigure: """Layers the provided figures. Be default, the layouts are sequentially applied, so the layouts of later figures will override the layouts of early @@ -589,7 +589,7 @@ def layer( Used to add any custom changes to the underlying plotly figure. Note that the existing data traces should not be removed. This may lead to unexpected behavior if traces are modified in a way that break data mappings. - overall_title: + title: Overall title to set for the figure. Returns: @@ -608,7 +608,7 @@ def layer( # remove the legend title as it is likely incorrect remove_legend_title=True, unsafe_update_figure=unsafe_update_figure, - overall_title=overall_title, + overall_title=title, ) exec_ctx = make_user_exec_ctx() diff --git a/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py b/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py index edebc75a5..2b93437eb 100644 --- a/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py +++ b/plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py @@ -352,13 +352,11 @@ def get_subplot_titles( for fig in fig_row ] elif subplot_titles is not None: - # Convert to list and adjust length + # Convert to list and truncate if needed titles = list(subplot_titles) total_subplots = rows * cols if len(titles) > total_subplots: return titles[:total_subplots] - elif len(titles) < total_subplots: - titles.extend([""] * (total_subplots - len(titles))) return titles return [] @@ -464,8 +462,6 @@ def atomic_make_subplots( cols, ) - return atomic_layer([""] * (total_subplots - len(final_subplot_titles))) - return atomic_layer( *[fig for fig_row in grid for fig in fig_row], specs=get_new_specs(