Skip to content

Commit

Permalink
pull in main
Browse files Browse the repository at this point in the history
  • Loading branch information
AkshatJawne committed Dec 5, 2024
2 parents 7f9f6a5 + ce3843a commit 3d0729c
Show file tree
Hide file tree
Showing 73 changed files with 11,979 additions and 1,696 deletions.
1 change: 1 addition & 0 deletions .github/workflows/release-python-package.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ jobs:
uses: pypa/gh-action-pypi-publish@release/v1
with:
packages-dir: plugins/${{ inputs.package }}/dist/
attestations: false # TODO: Followup this thread to see if there's a better fix https://github.com/pypa/gh-action-pypi-publish/issues/283

check-make-docs:
runs-on: ubuntu-22.04
Expand Down
1 change: 1 addition & 0 deletions jest.config.base.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const nodeModulesToTransform = [
'rehype.*',
'web-namespaces',
'hastscript',
'@astral-sh/ruff-wasm-web',
];

module.exports = {
Expand Down
11,353 changes: 9,723 additions & 1,630 deletions package-lock.json

Large diffs are not rendered by default.

20 changes: 20 additions & 0 deletions plugins/plotly-express/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,26 @@
All notable changes to this project will be documented in this file. See [conventional commits](https://www.conventionalcommits.org/) for commit guidelines.

- - -
## plotly-express-v0.12.0 - 2024-11-23
#### Bug Fixes
- `dx` now respects the webgl flag (#934) - (9cdf1ee) - Joe
- Remove `frequency_bar` (#955) - (17fbfca) - Joe
- Correct type for generated JsPlugin (#741) - (7da0ecc) - Joe
- Remove server startup from python tests (#768) - (c6c2dd2) - Joe
- Plotly express ticking 3d plots reset pending orientation on tick (#677) - (169354f) - Matthew Runyon
- Prevent pushing broken docs to main (#719) - (86fb7aa) - Joe
- Can't pass both x and y to violin, box and strip (#699) - (70c1805) - Joe
#### Build system
- Upgrade to Vite 5 (#899) - (e94b990) - Matthew Runyon
#### Documentation
- Mention Deephaven version where `server-ui` Docker image is mentioned (#951) - (1fac6af) - JJ Brosnan
#### Features
- Allow passing in a pandas dataframe to dx plots (#967) - (cf03ff0) - Joe
#### Tests
- default tox to 3.8 (#972) - (103c1e7) - Joe

- - -

## plotly-express-v0.11.2 - 2024-07-31
#### Bug Fixes
- Add hist by e2e test and fix error with static plot by (#664) - (88eeaea) - Joe
Expand Down
4 changes: 2 additions & 2 deletions plugins/plotly-express/setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name = deephaven-plugin-plotly-express
description = Deephaven Chart Plugin
long_description = file: README.md
long_description_content_type = text/markdown
version = 0.11.2.dev0
version = 0.12.0.dev0
url = https://github.com/deephaven/deephaven-plugins
project_urls =
Source Code = https://github.com/deephaven/deephaven-plugins
Expand All @@ -25,7 +25,7 @@ package_dir=
=src
packages=find_namespace:
install_requires =
deephaven-core>=0.36.0
deephaven-core>=0.37.0
deephaven-plugin>=0.6.0
plotly
deephaven-plugin-utilities>=0.0.2
Expand Down
23 changes: 12 additions & 11 deletions plugins/plotly-express/src/js/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@deephaven/js-plugin-plotly-express",
"version": "0.11.2",
"version": "0.12.0",
"description": "Deephaven plotly express plugin",
"keywords": [
"Deephaven",
Expand Down Expand Up @@ -36,7 +36,8 @@
"update-dh-packages": "node ../../../../tools/update-dh-packages.mjs"
},
"devDependencies": {
"@deephaven/jsapi-types": "1.0.0-dev0.35.2",
"@deephaven/jsapi-types": "1.0.0-dev0.36.1",
"@deephaven/test-utils": "0.97.0",
"@types/deep-equal": "^1.0.1",
"@types/plotly.js": "^2.12.18",
"@types/plotly.js-dist-min": "^2.3.1",
Expand All @@ -51,15 +52,15 @@
"react-dom": "^17.0.2"
},
"dependencies": {
"@deephaven/chart": "0.88.0",
"@deephaven/components": "0.88.0",
"@deephaven/dashboard": "0.88.0",
"@deephaven/dashboard-core-plugins": "0.88.0",
"@deephaven/icons": "0.88.0",
"@deephaven/jsapi-bootstrap": "0.88.0",
"@deephaven/log": "0.88.0",
"@deephaven/plugin": "0.88.0",
"@deephaven/utils": "0.88.0",
"@deephaven/chart": "0.97.0",
"@deephaven/components": "0.97.0",
"@deephaven/dashboard": "0.97.0",
"@deephaven/dashboard-core-plugins": "0.97.0",
"@deephaven/icons": "0.97.0",
"@deephaven/jsapi-bootstrap": "0.97.0",
"@deephaven/log": "0.97.0",
"@deephaven/plugin": "0.97.0",
"@deephaven/utils": "0.97.0",
"deep-equal": "^2.2.1",
"nanoid": "^5.0.7",
"plotly.js": "^2.29.1",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { Layout } from 'plotly.js';
import { dh as DhType } from '@deephaven/jsapi-types';
import { TestUtils } from '@deephaven/utils';
import { TestUtils } from '@deephaven/test-utils';
import { ChartModel } from '@deephaven/chart';
import { PlotlyExpressChartModel } from './PlotlyExpressChartModel';
import { PlotlyChartWidgetData } from './PlotlyExpressChartUtils';
Expand Down Expand Up @@ -262,4 +262,62 @@ describe('PlotlyExpressChartModel', () => {
new CustomEvent(ChartModel.EVENT_DOWNSAMPLEFAILED)
);
});

it('should swap replaceable WebGL traces without blocker events if WebGL is disabled or reenabled', async () => {
const mockWidget = createMockWidget([SMALL_TABLE], 'scattergl');
const chartModel = new PlotlyExpressChartModel(
mockDh,
mockWidget,
jest.fn()
);

const mockSubscribe = jest.fn();
await chartModel.subscribe(mockSubscribe);
await new Promise(process.nextTick); // Subscribe is async
chartModel.setRenderOptions({ webgl: true });
expect(chartModel.plotlyData[0].type).toBe('scattergl');
chartModel.setRenderOptions({ webgl: false });
expect(chartModel.plotlyData[0].type).toBe('scatter');
chartModel.setRenderOptions({ webgl: true });
expect(chartModel.plotlyData[0].type).toBe('scattergl');

// No events should be emitted since the trace is replaceable
expect(mockSubscribe).toHaveBeenCalledTimes(0);
});

it('should emit blocker events only if unreplaceable WebGL traces are present and WebGL is disabled, then blocker clear events when reenabled', async () => {
const mockWidget = createMockWidget([SMALL_TABLE], 'scatter3d');
const chartModel = new PlotlyExpressChartModel(
mockDh,
mockWidget,
jest.fn()
);

const mockSubscribe = jest.fn();
await chartModel.subscribe(mockSubscribe);
await new Promise(process.nextTick); // Subscribe is async
chartModel.setRenderOptions({ webgl: true });
// no calls because the chart has webgl enabled
expect(mockSubscribe).toHaveBeenCalledTimes(0);
chartModel.setRenderOptions({ webgl: false });
// blocking event should be emitted
expect(mockSubscribe).toHaveBeenCalledTimes(1);
expect(mockSubscribe).toHaveBeenLastCalledWith(
new CustomEvent(ChartModel.EVENT_BLOCKER)
);
chartModel.setRenderOptions({ webgl: true });
// blocking clear event should be emitted, but this doesn't count as an acknowledge
expect(mockSubscribe).toHaveBeenCalledTimes(2);
expect(mockSubscribe).toHaveBeenLastCalledWith(
new CustomEvent(ChartModel.EVENT_BLOCKER_CLEAR)
);
expect(chartModel.hasAcknowledgedWebGlWarning).toBe(false);
// if user had accepted the rendering (simulated by fireBlockerClear), no EVENT_BLOCKER event should be emitted again
chartModel.fireBlockerClear();
chartModel.setRenderOptions({ webgl: false });
expect(mockSubscribe).toHaveBeenCalledTimes(3);
expect(mockSubscribe).toHaveBeenLastCalledWith(
new CustomEvent(ChartModel.EVENT_BLOCKER_CLEAR)
);
});
});
55 changes: 55 additions & 0 deletions plugins/plotly-express/src/js/src/PlotlyExpressChartModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,22 @@ import type { Layout, Data, PlotData, LayoutAxis } from 'plotly.js';
import type { dh as DhType } from '@deephaven/jsapi-types';
import { ChartModel, ChartUtils } from '@deephaven/chart';
import Log from '@deephaven/log';
import { RenderOptions } from '@deephaven/chart/dist/ChartModel';
import {
DownsampleInfo,
PlotlyChartWidgetData,
areSameAxisRange,
downsample,
getDataMappings,
getPathParts,
getReplaceableWebGlTraceIndices,
getWidgetData,
isAutoAxis,
isLineSeries,
isLinearAxis,
removeColorsFromData,
setWebGlTraceType,
hasUnreplaceableWebGlTraces,
} from './PlotlyExpressChartUtils';

const log = Log.module('@deephaven/js-plugin-plotly-express.ChartModel');
Expand Down Expand Up @@ -116,6 +120,17 @@ export class PlotlyExpressChartModel extends ChartModel {

isDownsamplingDisabled = false;

/**
* Set of traces that are originally WebGL and can be replaced with non-WebGL traces.
* These need to be replaced if WebGL is disabled and re-enabled if WebGL is enabled again.
*/
webGlTraceIndices: Set<number> = new Set();

/**
* The WebGl warning is only shown once per chart. When the user acknowledges the warning, it will not be shown again.
*/
hasAcknowledgedWebGlWarning = false;

override getData(): Partial<Data>[] {
const hydratedData = [...this.plotlyData];

Expand Down Expand Up @@ -207,6 +222,41 @@ export class PlotlyExpressChartModel extends ChartModel {
this.widget = undefined;
}

override setRenderOptions(renderOptions: RenderOptions): void {
this.handleWebGlAllowed(renderOptions.webgl, this.renderOptions?.webgl);
super.setRenderOptions(renderOptions);
}

/**
* Handle the WebGL option being set in the render options.
* If WebGL is enabled, traces have their original types as given.
* If WebGL is disabled, replace traces that require WebGL with non-WebGL traces if possible.
* Also, show a dismissible warning per-chart if there are WebGL traces that cannot be replaced.
* @param webgl The new WebGL value. True if WebGL is enabled.
* @param prevWebgl The previous WebGL value
*/
handleWebGlAllowed(webgl = true, prevWebgl = true): void {
setWebGlTraceType(this.plotlyData, webgl, this.webGlTraceIndices);

const needsBlocker = hasUnreplaceableWebGlTraces(this.plotlyData);

// If WebGL is disabled and there are traces that require WebGL, show a warning that is dismissible on a per-chart basis
if (needsBlocker && !webgl && !this.hasAcknowledgedWebGlWarning) {
this.fireBlocker([
'WebGL is disabled but this chart cannot render without it. Check the Advanced section in the settings to enable WebGL or click below to render with WebGL for this chart.',
]);
} else if (webgl && !prevWebgl && needsBlocker) {
// clear the blocker but not the acknowledged flag in case WebGL is disabled again
this.fireBlockerClear(false);
}
}

override fireBlockerClear(isAcknowledged = true): void {
super.fireBlockerClear();
this.hasAcknowledgedWebGlWarning =
isAcknowledged || this.hasAcknowledgedWebGlWarning;
}

updateLayout(data: PlotlyChartWidgetData): void {
const { figure } = data;
const { plotly } = figure;
Expand Down Expand Up @@ -246,6 +296,11 @@ export class PlotlyExpressChartModel extends ChartModel {
);
}

// Retrieve the indexes of traces that require WebGL so they can be replaced if WebGL is disabled
this.webGlTraceIndices = getReplaceableWebGlTraceIndices(this.plotlyData);

this.handleWebGlAllowed(this.renderOptions?.webgl);

newReferences.forEach(async (id, i) => {
this.tableDataMap.set(id, {}); // Plot may render while tables are being fetched. Set this to avoid a render error
const table = (await references[i].fetch()) as DhType.Table;
Expand Down
116 changes: 116 additions & 0 deletions plugins/plotly-express/src/js/src/PlotlyExpressChartUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ import {
removeColorsFromData,
getDataMappings,
PlotlyChartWidgetData,
getReplaceableWebGlTraceIndices,
hasUnreplaceableWebGlTraces,
setWebGlTraceType,
} from './PlotlyExpressChartUtils';

describe('getDataMappings', () => {
Expand Down Expand Up @@ -202,3 +205,116 @@ describe('areSameAxisRange', () => {
expect(areSameAxisRange(null, [0, 10])).toBe(false);
});
});

describe('getReplaceableWebGlTraceIndexes', () => {
it('should return the indexes of any trace with gl', () => {
expect(
getReplaceableWebGlTraceIndices([
{
type: 'scattergl',
},
{
type: 'scatter',
},
{
type: 'scatter3d',
},
{
type: 'scattergl',
},
])
).toEqual(new Set([0, 3]));
});

it('should return an empty set if there are no traces with gl', () => {
expect(
getReplaceableWebGlTraceIndices([
{
type: 'scatter',
},
{
type: 'scatter3d',
},
])
).toEqual(new Set());
});
});

describe('hasUnreplaceableWebGlTraces', () => {
it('should return true if there is a single unreplaceable trace', () => {
expect(
hasUnreplaceableWebGlTraces([
{
type: 'scattermapbox',
},
])
).toBe(true);
});

it('should return true if there are unreplaceable traces', () => {
expect(
hasUnreplaceableWebGlTraces([
{
type: 'scattergl',
},
{
type: 'scatter',
},
{
type: 'scatter3d',
},
{
type: 'scattergl',
},
{
type: 'scatter3d',
},
])
).toBe(true);
});

it('should return false if there are no unreplaceable traces', () => {
expect(
hasUnreplaceableWebGlTraces([
{
type: 'scatter',
},
{
type: 'scattergl',
},
])
).toBe(false);
});
});

describe('setWebGlTraceType', () => {
it('should set the trace type to gl if webgl is enabled', () => {
const data: Plotly.Data[] = [
{
type: 'scatter',
},
{
type: 'scatter',
},
];
const webGlTraceIndices = new Set([1]);
setWebGlTraceType(data, true, webGlTraceIndices);
expect(data[0].type).toBe('scatter');
expect(data[1].type).toBe('scattergl');
});

it('should remove the gl from the trace type if webgl is disabled', () => {
const data: Plotly.Data[] = [
{
type: 'scatter',
},
{
type: 'scattergl',
},
];
const webGlTraceIndices = new Set([1]);
setWebGlTraceType(data, false, webGlTraceIndices);
expect(data[0].type).toBe('scatter');
expect(data[1].type).toBe('scatter');
});
});
Loading

0 comments on commit 3d0729c

Please sign in to comment.