Skip to content

Commit 287266f

Browse files
Copilotjnumainville
andcommitted
Address code review feedback: improve None handling and add title truncation logic
Co-authored-by: jnumainville <[email protected]>
1 parent 6292cc4 commit 287266f

File tree

2 files changed

+28
-3
lines changed

2 files changed

+28
-3
lines changed

plugins/plotly-express/src/deephaven/plot/express/plots/subplots.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -398,16 +398,21 @@ def atomic_make_subplots(
398398
final_subplot_titles.append("")
399399
else:
400400
extracted_title = extract_title_from_figure(fig)
401+
# Explicitly check for None to handle empty strings correctly
401402
final_subplot_titles.append(
402-
extracted_title if extracted_title else ""
403+
extracted_title if extracted_title is not None else ""
403404
)
404405
elif subplot_titles is not None:
405406
# Convert to list if tuple
406407
final_subplot_titles = list(subplot_titles)
407408

408-
# Pad with empty strings if needed
409+
# Validate and adjust length if needed
409410
total_subplots = rows * cols
410-
if len(final_subplot_titles) < total_subplots:
411+
if len(final_subplot_titles) > total_subplots:
412+
# Truncate if too many titles provided
413+
final_subplot_titles = final_subplot_titles[:total_subplots]
414+
elif len(final_subplot_titles) < total_subplots:
415+
# Pad with empty strings if too few titles provided
411416
final_subplot_titles.extend(
412417
[""] * (total_subplots - len(final_subplot_titles))
413418
)

plugins/plotly-express/test/deephaven/plot/express/plots/test_make_subplots.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -715,6 +715,26 @@ def test_make_subplots_conflicting_title_options(self):
715715

716716
self.assertIn("Cannot use both", str(context.exception))
717717

718+
def test_make_subplots_with_too_many_subplot_titles(self):
719+
import src.deephaven.plot.express as dx
720+
721+
chart = dx.scatter(self.source, x="X", y="Y")
722+
# Provide more titles than subplots - should be truncated
723+
charts = dx.make_subplots(
724+
chart,
725+
chart,
726+
rows=2,
727+
subplot_titles=["Plot 1", "Plot 2", "Plot 3", "Plot 4"],
728+
).to_dict(self.exporter)
729+
730+
# Check that only 2 annotations were created (truncated)
731+
layout = charts["plotly"]["layout"]
732+
self.assertIn("annotations", layout)
733+
annotations = layout["annotations"]
734+
self.assertEqual(len(annotations), 2)
735+
self.assertEqual(annotations[0]["text"], "Plot 1")
736+
self.assertEqual(annotations[1]["text"], "Plot 2")
737+
718738

719739
if __name__ == "__main__":
720740
unittest.main()

0 commit comments

Comments
 (0)