Skip to content

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

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

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open
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
279 changes: 218 additions & 61 deletions package-lock.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion plugins/plotly-express/setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ packages=find_namespace:
install_requires =
deephaven-core>=0.37.6
deephaven-plugin>=0.6.0
plotly>=5.15.0,<6.0.0
plotly>=6.0.0,<7.0.0
deephaven-plugin-utilities>=0.0.2
typing_extensions;python_version<'3.11'
jpy
Expand Down
3 changes: 3 additions & 0 deletions plugins/plotly-express/src/deephaven/plot/express/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@
line_mapbox,
density_heatmap,
indicator,
scatter_map,
density_map,
line_map,
)

from .data import data_generators
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@ def draw_finance(
data.append(
go_func(
x=data_frame[x_f],
open=data_frame[open],
high=data_frame[high],
low=data_frame[low],
close=data_frame[close],
open=data_frame[o],
high=data_frame[h],
low=data_frame[l],
close=data_frame[c],
)
)

Expand Down Expand Up @@ -200,7 +200,7 @@ def draw_density_heatmap(

heatmap.update_layout(
coloraxis1=coloraxis_layout,
title=title,
title_text=title,
template=template,
xaxis_title=x,
yaxis_title=y,
Expand Down Expand Up @@ -276,7 +276,7 @@ def draw_indicator(
fig.update_traces(delta_reference=data_frame[reference][0])

if layout_title:
fig.update_layout(title=layout_title)
fig.update_layout(title_text=layout_title)

if title:
# This is the title on the indicator trace. This is where it should go by default.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@
from .pie import pie
from ._layer import layer
from .subplots import make_subplots
from .maps import scatter_geo, scatter_mapbox, density_mapbox, line_geo, line_mapbox
from .maps import (
scatter_geo,
scatter_mapbox,
density_mapbox,
line_geo,
line_mapbox,
scatter_map,
density_map,
line_map,
)
from .heatmap import density_heatmap
from .indicator import indicator
85 changes: 67 additions & 18 deletions plugins/plotly-express/src/deephaven/plot/express/plots/maps.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

from typing import Callable
import warnings

from plotly import express as px

Expand Down Expand Up @@ -148,7 +149,55 @@ def scatter_geo(
return process_args(args, {"scatter"}, px_func=px.scatter_geo)


def scatter_mapbox(
def scatter_mapbox(*args, **kwargs) -> DeephavenFigure:
"""
c
"""
warnings.warn(
"scatter_mapbox is deprecated and will be removed in a future release. Use scatter_map instead.",
DeprecationWarning,
stacklevel=2,
)

if "style_mapbox" in kwargs:
kwargs["map_style"] = kwargs.pop("style_mapbox")

return scatter_map(*args, **kwargs)


def line_mapbox(*args, **kwargs) -> DeephavenFigure:
"""
Deprecated function. Use line_map instead.
"""
warnings.warn(
"line_mapbox is deprecated and will be removed in a future release. Use line_map instead.",
DeprecationWarning,
stacklevel=2,
)

if "style_mapbox" in kwargs:
kwargs["map_style"] = kwargs.pop("style_mapbox")

return line_map(*args, **kwargs)


def density_mapbox(*args, **kwargs) -> DeephavenFigure:
"""
Deprecated function. Use density_map instead.
"""
warnings.warn(
"density_mapbox is deprecated and will be removed in a future release. Use density_map instead.",
DeprecationWarning,
stacklevel=2,
)

if "style_mapbox" in kwargs:
kwargs["map_style"] = kwargs.pop("style_mapbox")

return density_map(*args, **kwargs)


def scatter_map(
table: PartitionableTableLike,
lat: str | None = None,
lon: str | None = None,
Expand Down Expand Up @@ -178,13 +227,13 @@ def scatter_mapbox(
opacity: float | None = None,
zoom: float | None = None,
center: dict[str, float] | None = None,
mapbox_style: str = "open-street-map",
map_style: str = "open-street-map",
title: str | None = None,
template: str | None = None,
unsafe_update_figure: Callable = default_callback,
) -> DeephavenFigure:
"""
Create a scatter_mapbox plot
Create a scatter_map plot

Args:
table: A table to pull data from.
Expand Down Expand Up @@ -241,7 +290,7 @@ def scatter_mapbox(
center: A dictionary of center coordinates.
The keys should be 'lat' and 'lon' and the values should be floats
that represent the lat and lon of the center of the map.
mapbox_style: The style of the map.
map_style: The style of the map.
One of 'open-street-map', 'white-bg', 'carto-positron', 'carto-darkmatter',
and 'stamen-terrain', 'stamen-toner', 'stamen-watercolor'
title: The title of the chart
Expand All @@ -255,11 +304,11 @@ def scatter_mapbox(
mappings.

Returns:
DeephavenFigure: A DeephavenFigure that contains the scatter_mapbox figure
DeephavenFigure: A DeephavenFigure that contains the scatter_map figure
"""
args = locals()

return process_args(args, {"scatter"}, px_func=px.scatter_mapbox)
return process_args(args, {"scatter"}, px_func=px.scatter_map)


def line_geo(
Expand Down Expand Up @@ -408,7 +457,7 @@ def line_geo(
return process_args(args, {"line"}, px_func=px.line_geo)


def line_mapbox(
def line_map(
table: PartitionableTableLike,
lat: str | None = None,
lon: str | None = None,
Expand All @@ -433,14 +482,14 @@ def line_mapbox(
width_sequence: list[int] | None = None,
width_map: dict[str | tuple[str], str] | None = None,
zoom: float | None = None,
mapbox_style: str = "open-street-map",
map_style: str = "open-street-map",
center: dict[str, float] | None = None,
title: str | None = None,
template: str | None = None,
unsafe_update_figure: Callable = default_callback,
) -> DeephavenFigure:
"""
Create a line_mapbox plot
Create a line_map plot

Args:
table: A table to pull data from.
Expand Down Expand Up @@ -506,7 +555,7 @@ def line_mapbox(
center: A dictionary of center coordinates.
The keys should be 'lat' and 'lon' and the values should be floats
that represent the lat and lon of the center of the map.
mapbox_style: The style of the map.
map_style: The style of the map.
One of 'open-street-map', 'white-bg', 'carto-positron', 'carto-darkmatter',
and 'stamen-terrain', 'stamen-toner', 'stamen-watercolor'
title: The title of the chart
Expand All @@ -520,15 +569,15 @@ def line_mapbox(
mappings.

Returns:
DeephavenFigure: A DeephavenFigure that contains the line_mapbox figure
DeephavenFigure: A DeephavenFigure that contains the line_map figure

"""
args = locals()

return process_args(args, {"line"}, px_func=px.line_mapbox)
return process_args(args, {"line"}, px_func=px.line_map)


def density_mapbox(
def density_map(
table: TableLike,
lat: str | None = None,
lon: str | None = None,
Expand All @@ -542,13 +591,13 @@ def density_mapbox(
opacity: float | None = None,
zoom: float | None = None,
center: dict[str, float] | None = None,
mapbox_style: str = "open-street-map",
map_style: str = "open-street-map",
title: str | None = None,
template: str | None = None,
unsafe_update_figure: Callable = default_callback,
) -> DeephavenFigure:
"""
Create a density_mapbox plot
Create a density_map plot

Args:
table: A table to pull data from.
Expand All @@ -567,7 +616,7 @@ def density_mapbox(
center: A dictionary of center coordinates.
The keys should be 'lat' and 'lon' and the values should be floats
that represent the lat and lon of the center of the map.
mapbox_style: The style of the map.
map_style: The style of the map.
One of 'open-street-map', 'white-bg', 'carto-positron', 'carto-darkmatter',
and 'stamen-terrain', 'stamen-toner', 'stamen-watercolor'
title: The title of the chart
Expand All @@ -581,8 +630,8 @@ def density_mapbox(
mappings.

Returns:
DeephavenFigure: A DeephavenFigure that contains the density_mapbox figure
DeephavenFigure: A DeephavenFigure that contains the density_map figure
"""
args = locals()

return process_args(args, set(), px_func=px.density_mapbox)
return process_args(args, set(), px_func=px.density_map)
6 changes: 3 additions & 3 deletions plugins/plotly-express/src/js/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
"@deephaven/jsapi-types": "1.0.0-dev0.38.0",
"@deephaven/test-utils": "0.105.0",
"@types/deep-equal": "^1.0.1",
"@types/plotly.js": "^2.12.18",
"@types/plotly.js": "^3.0.0",
"@types/plotly.js-dist-min": "^2.3.1",
"@types/react": "^17.0.2",
"@types/react-plotly.js": "^2.6.0",
Expand All @@ -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",
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

"plotly.js-dist-min": "^3.0.0",
"react-plotly.js": "^2.4.0",
"react-redux": "^7.2.9"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ jest.mock('./PlotlyExpressChartUtils', () => ({
function createMockWidget(
tables: DhType.Table[],
plotType = 'scatter',
title: string | Partial<Layout['title']> = 'Test'
title: Partial<Layout['title']> = { text: "Title" }
) {
const layoutAxes: Partial<Layout> = {};
tables.forEach((_, i) => {
Expand Down
14 changes: 6 additions & 8 deletions plugins/plotly-express/src/js/src/PlotlyExpressChartModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@
*/
dataTypeMap: Map<string, string> = new Map();

override getData(): Partial<Data>[] {

Check failure on line 166 in plugins/plotly-express/src/js/src/PlotlyExpressChartModel.ts

View workflow job for this annotation

GitHub Actions / test-js / unit

Property 'getData' in type 'PlotlyExpressChartModel' is not assignable to the same property in base type 'ChartModel'.
const hydratedData = [...this.plotlyData];

this.tableColumnReplacementMap.forEach((columnReplacements, tableId) => {
Expand Down Expand Up @@ -199,7 +199,7 @@
return hydratedData;
}

override getLayout(): Partial<Layout> {

Check failure on line 202 in plugins/plotly-express/src/js/src/PlotlyExpressChartModel.ts

View workflow job for this annotation

GitHub Actions / test-js / unit

Property 'getLayout' in type 'PlotlyExpressChartModel' is not assignable to the same property in base type 'ChartModel'.
return this.layout;
}

Expand Down Expand Up @@ -389,7 +389,7 @@
);

if (layoutUpdate) {
this.fireLayoutUpdated(layoutUpdate);

Check failure on line 392 in plugins/plotly-express/src/js/src/PlotlyExpressChartModel.ts

View workflow job for this annotation

GitHub Actions / test-js / unit

Argument of type 'Partial<Plotly.Layout>' is not assignable to parameter of type 'Partial<import("/home/runner/work/deephaven-plugins/deephaven-plugins/node_modules/@types/plotly.js/index").Layout>'.
}
}

Expand Down Expand Up @@ -773,7 +773,7 @@

override setDimensions(rect: DOMRect): void {
super.setDimensions(rect);
ChartUtils.getLayoutRanges(this.layout);

Check failure on line 776 in plugins/plotly-express/src/js/src/PlotlyExpressChartModel.ts

View workflow job for this annotation

GitHub Actions / test-js / unit

Argument of type 'Partial<Plotly.Layout>' is not assignable to parameter of type 'Partial<import("/home/runner/work/deephaven-plugins/deephaven-plugins/node_modules/@types/plotly.js/index").Layout>'.
this.downsampleMap.forEach((_, id) => {
this.updateDownsampledTable(id);
});
Expand All @@ -791,24 +791,22 @@
}

shouldPauseOnUserInteraction(): boolean {
return (
this.hasScene() || this.hasGeo() || this.hasMapbox() || this.hasPolar()
);
return this.hasScene() || this.hasGeo() || this.hasMap() || this.hasPolar();
}

hasScene(): boolean {
private hasScene(): boolean {
return this.plotlyData.some(d => 'scene' in d && d.scene != null);
}

hasGeo(): boolean {
private hasGeo(): boolean {
return this.plotlyData.some(d => 'geo' in d && d.geo != null);
}

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

hasPolar(): boolean {
private hasPolar(): boolean {
return this.plotlyData.some(({ type }) => type?.includes('polar'));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ describe('hasUnreplaceableWebGlTraces', () => {
expect(
hasUnreplaceableWebGlTraces([
{
type: 'scattermapbox',
type: 'scatter3d',
},
])
).toBe(true);
Expand Down
6 changes: 3 additions & 3 deletions plugins/plotly-express/src/js/src/PlotlyExpressChartUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ const UNREPLACEABLE_WEBGL_TRACE_TYPES = new Set([
'mesh3d',
'cone',
'streamtube',
'scattermapbox',
'choroplethmapbox',
'densitymapbox',
'scattermap',
'choroplethmap',
'densitymap',
]);

/*
Expand Down
10 changes: 6 additions & 4 deletions plugins/plotly-express/test/deephaven/plot/express/BaseTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@
from unittest.mock import patch

import pandas as pd
from deephaven import DHError
from deephaven.plot.express import DeephavenFigure
from typing import List, Any
import os
import pathlib
from typing import List

# Deephaven's NULL_INT and NULL_DOUBLE, converted with Plotly's base64 API
# https://github.com/plotly/plotly.py/pull/4470
PLOTLY_NULL_INT = {"dtype": "i4", "bdata": "AAAAgA=="}
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added comment to code

PLOTLY_NULL_DOUBLE = {"dtype": "f8", "bdata": "////////7/8="}


def remap_types(
Expand Down
Loading
Loading