Skip to content
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

[POC] add echarts scatter #8229

Closed
wants to merge 1 commit into from
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ const DEFAULT_ORDER = [
'mapbox', 'kepler', 'cal_heatmap', 'rose', 'bubble', 'deck_geojson',
'horizon', 'markup', 'deck_multi', 'compare', 'partition', 'event_flow',
'deck_path', 'directed_force', 'world_map', 'paired_ttest', 'para',
'iframe', 'country_map',
'iframe', 'country_map', 'echarts_scatter',
];

const typesWithDefaultOrder = new Set(DEFAULT_ORDER);
Expand Down
69 changes: 69 additions & 0 deletions superset/assets/src/explore/controlPanels/EchartsScatter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/**
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be part of the plugin.

* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { t } from '@superset-ui/translation';

export default {
label: t('Bubble Chart'),
controlPanelSections: [
{
label: t('Query'),
expanded: true,
controlSetRows: [
['series', 'entity'],
['x'],
['y'],
['adhoc_filters'],
['size'],
['max_bubble_size'],
['limit', null],
],
},
{
label: t('Chart Options'),
expanded: true,
controlSetRows: [
['color_scheme', 'label_colors'],
['show_legend', null],
],
},
{
label: t('X Axis'),
expanded: true,
controlSetRows: [
['x_axis_label', 'left_margin'],
['x_axis_format', 'x_ticks_layout'],
['x_log_scale', 'x_axis_showminmax'],
],
},
{
label: t('Y Axis'),
expanded: true,
controlSetRows: [
['y_axis_label', 'bottom_margin'],
['y_axis_format', null],
['y_log_scale', 'y_axis_showminmax'],
],
},
],
controlOverrides: {
color_scheme: {
renderTrigger: false,
},
},
};
2 changes: 2 additions & 0 deletions superset/assets/src/explore/controlPanels/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import BigNumber from './BigNumber';
import BigNumberTotal from './BigNumberTotal';
import BoxPlot from './BoxPlot';
import Bubble from './Bubble';
import EchartsScatter from './EchartsScatter';
Copy link
Contributor

Choose a reason for hiding this comment

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

This file is outdated. This is not necessary anymore.

import Bullet from './Bullet';
import CalHeatmap from './CalHeatmap';
import Chord from './Chord';
Expand Down Expand Up @@ -80,6 +81,7 @@ export const controlPanelConfigs = extraOverrides({
big_number_total: BigNumberTotal,
box_plot: BoxPlot,
bubble: Bubble,
echarts_scatter: EchartsScatter,
bullet: Bullet,
cal_heatmap: CalHeatmap,
chord: Chord,
Expand Down
2 changes: 2 additions & 0 deletions superset/assets/src/visualizations/presets/MainPreset.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import ForceDirectedChartPlugin from '@superset-ui/legacy-plugin-chart-force-dir
import HeatmapChartPlugin from '@superset-ui/legacy-plugin-chart-heatmap';
import HistogramChartPlugin from '@superset-ui/legacy-plugin-chart-histogram';
import HorizonChartPlugin from '@superset-ui/legacy-plugin-chart-horizon';
import EchartsScatterChartPlugin from '@superset-ui/superset-ui-plugin-echarts-scatter/src';
import IframeChartPlugin from '@superset-ui/legacy-plugin-chart-iframe';
import MapBoxChartPlugin from '@superset-ui/legacy-plugin-chart-map-box';
import MarkupChartPlugin from '@superset-ui/legacy-plugin-chart-markup';
Expand Down Expand Up @@ -102,6 +103,7 @@ export default class MainPreset extends Preset {
new TreemapChartPlugin().configure({ key: 'treemap' }),
new WordCloudChartPlugin().configure({ key: 'word_cloud' }),
new WorldMapChartPlugin().configure({ key: 'world_map' }),
new EchartsScatterChartPlugin().configure({ key: 'echarts_scatter' }),
],
});
}
Expand Down
9 changes: 9 additions & 0 deletions superset/viz.py
Original file line number Diff line number Diff line change
Expand Up @@ -2765,6 +2765,15 @@ def get_data(self, df):
return self.nest_values(levels)


class EchartsScatter(BubbleViz):
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you use the new api instead?

Copy link
Member

Choose a reason for hiding this comment

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

@kristw is there already a working plan for migrating some of the more complex legacy viz's to the new api, namely the ones that rely on Pandas functionality? Is the plan to do that logic in JS, och making the heavy lifting in the backend? I anticipate quite a bit of work there, and can lend a hand if necessary, especially if the new API needs new functionality..

Copy link
Contributor

@kristw kristw Sep 16, 2019

Choose a reason for hiding this comment

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

We had light discussions about listing postprocessing operations as a new field in the api/v1/query parameters to call those pandas functionality in addition to the simple table query that it already support, but none had the cycle to draft the API spec nor work on it yet. If you would like to step in, that could be helpful.

I was thinking something along the line of

const queryFormData = {
  group_by,
  adhoc_filters, 
  ...etc.
  // Add a new field that takes an array of post-processing operations
  // which we could enforce the schema in typescript 
  // then you need to modify /api/v1/query in python to handle it.
  postProcessing = [ { operation: '', options: {...} } ];
}

See superset-ui/query for current QueryFormData schema.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds reasonable. In all, I think this will be a big feature to implement, but can probably be dissected into smaller parts. I would probably start with implementing the pivot operation, as it is used quite frequently in viz's and should be easy to start with. I'm fairly overloaded with other stuff right now, but should be able to look into this in a few weeks time if nobody wants to take the lead.

Copy link
Contributor

Choose a reason for hiding this comment

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

Has anyone looked at this? Not sure if it is suitable here https://github.com/nickslevine/zebras

Copy link
Member

Choose a reason for hiding this comment

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

It also doesn't have to be either done in the frontend or backend; can be both. Some postprocessing can be very expensive to move to the front, especially percentiles/quantiles and the like that require having a full distribution, so those would be preferable to perform in the backend, while others might be well suited for the frontend. So I think being able to offer at least some postprocessing options in the backend would be nice, leaving it up to the viz plugin developer to decide where to perform the calculations.


"""Based on the NVD3 bubble chart"""

viz_type = "echarts_scatter"
verbose_name = _("Bubble Chart")
is_timeseries = False


viz_types = {
o.viz_type: o
for o in globals().values()
Expand Down