fix: DH-18653: migrate to plotly >= 6.0.0 #1179

jnumainville · 2025-05-08T17:21:33Z

Fixes #946

Bump plotly.py to 6.0.0
Bump plotly.js to 3.0.0
Deprecate any mapbox reference
Remove alignmentgroup and offsetgroup in tests where they now do not need to be set
Generally replaced NULL_INT and NULL_DOUBLE with a binary data form plotly is now converting those values to in tests
Replaced all title=<str> with title_text=<str> as the former is deprecated
Fixed some minor inconsistencies
Added a few e2e tests where I could to make sure migration did not break the charts. Those images were generated on main originally.

github-actions · 2025-05-08T17:25:20Z

plotly-express docs preview (Available for 14 days)

github-actions · 2025-05-09T15:36:53Z

plotly-express docs preview (Available for 14 days)

github-actions · 2025-05-09T15:42:44Z

plotly-express docs preview (Available for 14 days)

github-actions · 2025-05-09T20:50:14Z

plotly-express docs preview (Available for 14 days)

github-actions · 2025-05-09T21:34:53Z

plotly-express docs preview (Available for 14 days)

Copilot

Pull Request Overview

This PR migrates the plotting functionality to use plotly versions ≥6.0.0 by updating dependencies, replacing deprecated mapbox‐based functions with their new map counterparts, and updating test constants and parameter names.

Bump plotly.py to 6.0.0 and plotly.js to 3.0.0 in package configurations
Replace all usages of mapbox functions (e.g. scatter_mapbox, line_mapbox, density_mapbox) with new functions (scatter_map, line_map, density_map) and add deprecation warnings
Update test files to use new binary data constants (PLOTLY_NULL_INT and PLOTLY_NULL_DOUBLE) and adjust expected layouts accordingly

Reviewed Changes

Copilot reviewed 46 out of 46 changed files in this pull request and generated no comments.

File	Description
plugins/plotly-express/test/deephaven/plot/express/plots/*	Replace deprecated NULL_* values with new PLOTLY_NULL_* constants and update test names for map-based functions
plugins/plotly-express/src/js/*	Update JS utility and model files to work with the new plotly.js version and reframe map detection logic
plugins/plotly-express/src/deephaven/plot/express/plots/maps.py	Deprecate mapbox functions in favor of the new map functions; update parameter names (mapbox_style → map_style) and docstrings accordingly
plugins/plotly-express/setup.cfg & package.json	Bump plotly dependency constraints to support plotly version ≥6.0.0

Comments suppressed due to low confidence (3)

plugins/plotly-express/test/deephaven/plot/express/plots/test_indicator.py:236

For consistency across tests, consider replacing NULL_INT with PLOTLY_NULL_INT as done in other test files.

from deephaven.constants import NULL_INT

plugins/plotly-express/src/deephaven/plot/express/plots/maps.py:152

The deprecation functions now call the new scatter_map; ensure that the docstrings and return descriptions are updated to consistently reflect the new naming and API changes.

def scatter_mapbox(*args, **kwargs) -> DeephavenFigure:

plugins/plotly-express/src/js/src/PlotlyExpressChartModel.ts:794

Verify that checking for the substring 'map' in trace types (via hasMap()) does not unintentionally match trace types other than the intended map traces; a stricter condition may be needed.

return this.hasScene() || this.hasGeo() || this.hasMap() || this.hasPolar();

github-actions · 2025-05-13T19:24:49Z

plotly-express docs preview (Available for 14 days)

mofojed · 2025-05-14T15:53:42Z

plugins/plotly-express/src/js/package.json

@@ -68,8 +68,8 @@
    "deep-equal": "^2.2.1",
    "memoizee": "^0.4.17",
    "nanoid": "^5.0.7",
-    "plotly.js": "^2.29.1",
-    "plotly.js-dist-min": "^2.29.1",
+    "plotly.js": "^3.0.0",


We should update this in web-client-ui as well: https://github.com/deephaven/web-client-ui/blob/75fe9c11ee23e5fc59a1160750a28ae6c3f047b5/package.json#L96

deephaven/web-client-ui#2455

mofojed · 2025-05-14T15:58:00Z

plugins/plotly-express/src/js/src/PlotlyExpressChartModel.ts

+  hasMap(): boolean {
+    return this.plotlyData.some(({ type }) => type?.includes('map'));
  }


We should make all these has* methods private as well, since they're just used internally.

mofojed · 2025-05-14T16:01:43Z

plugins/plotly-express/test/deephaven/plot/express/BaseTest.py

+from typing import List
+
+
+PLOTLY_NULL_INT = {"dtype": "i4", "bdata": "AAAAgA=="}


Do you have any more details on these? Any documentation about where these are defined in plotly? I'm guessing you found these as a result of unit tests, but would like to know why these values.

It is this PR - plotly/plotly.py#4470
Essentially it is a custom base64 spec

More specifically PLOTLY_NULL_INT is just [NULL_INT] converted to their spec

Added comment to code

mofojed · 2025-05-14T16:04:48Z

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

+                "close": PLOTLY_NULL_INT,
+                "high": PLOTLY_NULL_INT,
+                "low": PLOTLY_NULL_INT,
+                "open": PLOTLY_NULL_INT,


This expects null now, when it expected a 2D array before?

The 2D array was a subtle bug I noticed when going through and doing this PLOTLY_NULL_INT work. See the changes to the function in custom_draw. It should have been a null int array in the first place.

mofojed · 2025-05-14T16:12:41Z

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

-            "mapbox": {
-                "center": {"lat": NULL_INT, "lon": NULL_INT},
+            "map": {
+                "center": {"lat": -2147483648.0, "lon": -2147483648.0},


Does this actually make sense? How come it's getting these values?

This should have been NULL_INT still, fixed.
As to why it is these values, it is because plotly sets the center to the mean of the values by default (see here)
Since the NULL_INT is the only data point passed to plotly for lat and lon, it become the center. It's then not converted to the base64 spec because it is not considered a data array.

mofojed · 2025-05-14T16:13:42Z

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

                "orientation": "h",
                "showlegend": False,
                "textposition": "auto",
-                "x": [0.0],
+                "x": {"bdata": "AA==", "dtype": "i1"},


Where'd this magic value come from?

This is the [0.0] value encoded in their base64 spec. I didn't create a variable for it because it is not reused.

github-actions · 2025-05-14T16:42:15Z

plotly-express docs preview (Available for 14 days)

github-actions · 2025-05-29T15:34:20Z

plotly-express docs preview (Available for 14 days)

github-actions · 2025-05-29T21:41:00Z

plotly-express docs preview (Available for 14 days)

github-actions · 2025-05-30T15:28:09Z

plotly-express docs preview (Available for 14 days)

jnumainville added 6 commits May 7, 2025 13:58

wip

c99d261

wip

b102a79

wip

6180e77

wip

64354b9

wip

d30f2d9

wip

194462e

wip

b9e05bc

wip

0251aa2

jnumainville added 2 commits May 9, 2025 14:33

wip

d1ab6b4

wip

f4c9c59

wip

12d7928

wip

60f6dbe

jnumainville requested review from Copilot and mofojed May 13, 2025 19:21

Copilot AI reviewed May 13, 2025

View reviewed changes

jnumainville marked this pull request as ready for review May 13, 2025 19:54

mofojed requested changes May 14, 2025

View reviewed changes

wip

5fe7b8e

jnumainville mentioned this pull request May 29, 2025

chore: update plotly version to 3.0.0 deephaven/web-client-ui#2455

Draft

wip

bd7b1ad

wip

da1e4a6

		from typing import List


		PLOTLY_NULL_INT = {"dtype": "i4", "bdata": "AAAAgA=="}

fix: DH-18653: migrate to plotly >= 6.0.0 #1179

Are you sure you want to change the base?

fix: DH-18653: migrate to plotly >= 6.0.0 #1179

Uh oh!

Conversation

jnumainville commented May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented May 8, 2025

Uh oh!

github-actions bot commented May 9, 2025

Uh oh!

github-actions bot commented May 9, 2025

Uh oh!

github-actions bot commented May 9, 2025

Uh oh!

github-actions bot commented May 9, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

github-actions bot commented May 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jnumainville May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jnumainville May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented May 14, 2025

Uh oh!

github-actions bot commented May 29, 2025

Uh oh!

github-actions bot commented May 29, 2025

Uh oh!

github-actions bot commented May 30, 2025

Uh oh!

Uh oh!

jnumainville commented May 8, 2025 •

edited

Loading

jnumainville May 14, 2025 •

edited

Loading

jnumainville May 14, 2025 •

edited

Loading