Skip to content

Commit

Permalink
[bugfix] making secondary_metric optional (#5682)
Browse files Browse the repository at this point in the history
I noticed that `secondary_metric` was required in the chart that users
it, namely sunburst, parallel coordinates, and world_map.

I set the control as allowing null, and found related bugs in the
process:
* parallel coordinates did not have support for the new MetricControl
* added color scheme support for parallel coordinates
* added option to set row_limit on these viz types
* sunburst to support numeric columns (number would show as blank)
  • Loading branch information
mistercrunch authored Aug 21, 2018
1 parent 8dfa565 commit 71e0c07
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 26 deletions.
2 changes: 2 additions & 0 deletions superset/assets/src/explore/controls.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,7 @@ export const controls = {
...metric,
label: t('Color Metric'),
default: null,
validators: [],
description: t('A metric to use for color'),
},
select_country: {
Expand Down Expand Up @@ -1437,6 +1438,7 @@ export const controls = {
type: 'CheckboxControl',
label: t('Data Table'),
default: false,
renderTrigger: true,
description: t('Whether to display the interactive data table'),
},

Expand Down
5 changes: 4 additions & 1 deletion superset/assets/src/explore/visTypes.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -1513,6 +1513,7 @@ export const visTypes = {
['country_fieldtype'],
['metric'],
['adhoc_filters'],
['row_limit'],
],
},
{
Expand Down Expand Up @@ -1592,13 +1593,15 @@ export const visTypes = {
['metrics'],
['secondary_metric'],
['adhoc_filters'],
['limit'],
['limit', 'row_limit'],
],
},
{
label: t('Options'),
expanded: true,
controlSetRows: [
['show_datatable', 'include_series'],
['linear_color_scheme'],
],
},
],
Expand Down
26 changes: 11 additions & 15 deletions superset/assets/src/visualizations/parallel_coordinates.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import d3 from 'd3';
import '../../vendor/parallel_coordinates/d3.parcoords.css';
import './parallel_coordinates.css';
import { colorScalerFactory } from '../modules/colors';

d3.parcoords = require('../../vendor/parallel_coordinates/d3.parcoords.js');
d3.divgrid = require('../../vendor/parallel_coordinates/divgrid.js');
Expand All @@ -12,29 +13,24 @@ function parallelCoordVis(slice, payload) {
const fd = slice.formData;
const data = payload.data;

let cols = fd.metrics;
const metrics = fd.metrics.map(m => m.label || m);

let cols = metrics;
if (fd.include_series) {
cols = [fd.series].concat(fd.metrics);
cols = [fd.series].concat(metrics);
}

const ttypes = {};
ttypes[fd.series] = 'string';
fd.metrics.forEach(function (v) {
metrics.forEach(function (v) {
ttypes[v] = 'number';
});

let ext = d3.extent(data, function (d) {
return d[fd.secondary_metric];
});
ext = [ext[0], (ext[1] - ext[0]) / 2, ext[1]];
const cScale = d3.scale.linear()
.domain(ext)
.range(['red', 'grey', 'blue'])
.interpolate(d3.interpolateLab);

const color = function (d) {
return cScale(d[fd.secondary_metric]);
};
const secondaryMetric = fd.secondary_metric ? fd.secondary_metric.label : fd.secondary_metric;
const colorScaler = fd.secondary_metric ?
colorScalerFactory(fd.linear_color_scheme, data, d => d[secondaryMetric]) :
() => 'grey';
const color = d => colorScaler(d[secondaryMetric]);
const container = d3.select(slice.selector);
container.selectAll('*').remove();
const effHeight = fd.show_datatable ? (slice.height() / 2) : slice.height();
Expand Down
4 changes: 2 additions & 2 deletions superset/assets/src/visualizations/sunburst.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import d3 from 'd3';
import { getColorFromScheme } from '../modules/colors';
import { wrapSvgText } from '../modules/utils';

require('./sunburst.css');
import './sunburst.css';

// Modified from http://bl.ocks.org/kerryrodden/7090426
function sunburstVis(slice, payload) {
Expand Down Expand Up @@ -250,7 +250,7 @@ function sunburstVis(slice, payload) {
let currentNode = root;
for (let level = 0; level < levels.length; level++) {
const children = currentNode.children || [];
const nodeName = levels[level];
const nodeName = levels[level].toString();
// If the next node has the name '0', it will
const isLeafNode = (level >= levels.length - 1) || levels[level + 1] === 0;
let childNode;
Expand Down
15 changes: 7 additions & 8 deletions superset/viz.py
Original file line number Diff line number Diff line change
Expand Up @@ -1727,8 +1727,6 @@ class WorldMapViz(BaseViz):

def query_obj(self):
qry = super(WorldMapViz, self).query_obj()
qry['metrics'] = [
self.form_data['metric'], self.form_data['secondary_metric']]
qry['groupby'] = [self.form_data['entity']]
return qry

Expand All @@ -1738,17 +1736,22 @@ def get_data(self, df):
cols = [fd.get('entity')]
metric = self.get_metric_label(fd.get('metric'))
secondary_metric = self.get_metric_label(fd.get('secondary_metric'))
columns = ['country', 'm1', 'm2']
if metric == secondary_metric:
ndf = df[cols]
# df[metric] will be a DataFrame
# because there are duplicate column names
ndf['m1'] = df[metric].iloc[:, 0]
ndf['m2'] = ndf['m1']
else:
cols += [metric, secondary_metric]
if secondary_metric:
cols += [metric, secondary_metric]
else:
cols += [metric]
columns = ['country', 'm1']
ndf = df[cols]
df = ndf
df.columns = ['country', 'm1', 'm2']
df.columns = columns
d = df.to_dict(orient='records')
for row in d:
country = None
Expand Down Expand Up @@ -1846,10 +1849,6 @@ class ParallelCoordinatesViz(BaseViz):
def query_obj(self):
d = super(ParallelCoordinatesViz, self).query_obj()
fd = self.form_data
d['metrics'] = copy.copy(fd.get('metrics'))
second = fd.get('secondary_metric')
if second not in d['metrics']:
d['metrics'] += [second]
d['groupby'] = [fd.get('series')]
return d

Expand Down

0 comments on commit 71e0c07

Please sign in to comment.