[GitHub] betodealmeida commented on a change in pull request #4800: Improve the calendar heatmap

2018-04-12 Thread GitBox
betodealmeida commented on a change in pull request #4800: Improve the calendar 
heatmap
URL: 
https://github.com/apache/incubator-superset/pull/4800#discussion_r181228982
 
 

 ##
 File path: superset/assets/javascripts/explore/stores/controls.jsx
 ##
 @@ -1003,6 +1010,46 @@ export const controls = {
 'relative to the time granularity selected'),
   },
 
+  cell_size: {
+type: 'TextControl',
+isInt: true,
+default: 10,
+validators: [v.integer],
+renderTrigger: true,
+label: t('Cell Size'),
+description: t('The size of the square cell, in pixels'),
+  },
+
+  cell_padding: {
+type: 'TextControl',
+isInt: true,
+validators: [v.integer],
+renderTrigger: true,
+default: 2,
+label: t('Cell Padding'),
+description: t('The distance between cells, in pixels'),
+  },
+
+  cell_radius: {
+type: 'TextControl',
+isInt: true,
+validators: [v.integer],
+renderTrigger: true,
+default: 2,
 
 Review comment:
   Argh, the black spots keep moving! :-P
   
   I think if we reduce to a small value they should go away, no? If not, I'm 
fine with 0.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] betodealmeida commented on a change in pull request #4800: Improve the calendar heatmap

2018-04-11 Thread GitBox
betodealmeida commented on a change in pull request #4800: Improve the calendar 
heatmap
URL: 
https://github.com/apache/incubator-superset/pull/4800#discussion_r180903916
 
 

 ##
 File path: superset/assets/javascripts/modules/colors.js
 ##
 @@ -122,6 +122,42 @@ export const spectrums = {
 '#FAFAFA',
 '#66',
   ],
+  greens: [
+'#cc',
+'#78c679',
+'#006837',
+  ],
+  purples: [
+'#f2f0f7',
+'#9e9ac8',
+'#54278f',
+  ],
+  oranges: [
+'#fef0d9',
+'#fc8d59',
+'#b3',
+  ],
+  red_yellow_blue: [
+'#d7191c',
+'#fdae61',
+'#bf',
+'#abd9e9',
+'#2c7bb6',
+  ],
+  brown_white_green: [
+'#a6611a',
+'#dfc27d',
+'#f5f5f5',
+'#80cdc1',
+'#018571',
+  ],
+  purple_white_green: [
 
 Review comment:
   Cool, #TIL.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] betodealmeida commented on a change in pull request #4800: Improve the calendar heatmap

2018-04-11 Thread GitBox
betodealmeida commented on a change in pull request #4800: Improve the calendar 
heatmap
URL: 
https://github.com/apache/incubator-superset/pull/4800#discussion_r180900714
 
 

 ##
 File path: superset/assets/visualizations/cal_heatmap.js
 ##
 @@ -1,38 +1,84 @@
-// JS
 import d3 from 'd3';
+import _ from 'underscore';
 
-// CSS
-require('./cal_heatmap.css');
-require('../node_modules/cal-heatmap/cal-heatmap.css');
+import { colorScalerFactory } from '../javascripts/modules/colors';
+import CalHeatMap from '../vendor/cal-heatmap/cal-heatmap';
+import '../vendor/cal-heatmap/cal-heatmap.css';
+import { d3TimeFormatPreset, d3FormatPreset } from 
'../javascripts/modules/utils';
+import './cal_heatmap.css';
+import { UTC } from '../javascripts/modules/dates';
 
-const CalHeatMap = require('cal-heatmap');
+const UTCTS = uts => UTC(new Date(uts)).getTime();
 
 function calHeatmap(slice, payload) {
-  const div = d3.select(slice.selector);
+  const fd = slice.formData;
+  const steps = fd.steps;
+  const valueFormatter = d3FormatPreset(fd.y_axis_format);
+  const timeFormatter = d3TimeFormatPreset(fd.x_axis_time_format);
+
+  const container = d3.select(slice.selector).style('height', slice.height());
+  container.selectAll('*').remove();
+  const div = container.append('div');
   const data = payload.data;
 
-  div.selectAll('*').remove();
-  const cal = new CalHeatMap();
+  const subDomainTextFormat = fd.show_values ? (date, value) => 
valueFormatter(value) : null;
+  const cellPadding = fd.cell_padding !== '' ? fd.cell_padding : 2;
+  const cellRadius = fd.cell_radius || 0;
+  const cellSize = fd.cell_size || 10;
+
+  // Trick to convert all timestamps to UTC
+  const metricsData = {};
+  Object.keys(data.data).forEach((metric) => {
+metricsData[metric] = {};
+Object.keys(data.data[metric]).forEach((ts) => {
+  metricsData[metric][UTCTS(ts * 1000) / 1000] = data.data[metric][ts];
+});
+  });
+
+  Object.keys(metricsData).forEach((metric) => {
+const calContainer = div.append('div');
+if (fd.show_metric_name) {
+  calContainer.append('h4').text(slice.verboseMetricName(metric));
+}
+const timestamps = metricsData[metric];
+const extents = d3.extent(Object.keys(timestamps), key => timestamps[key]);
+const step = (extents[1] - extents[0]) / (steps - 1);
+const colorScale = colorScalerFactory(fd.linear_color_scheme, null, null, 
extents);
+
+const legend = _.range(steps).map(i => extents[0] + (step * i));
 
 Review comment:
   This is more a general comment about our dependencies... some files use 
jquery, others use underscore. D3 provides a `range` function, maybe we could 
use it here and get rid of the underscore dependency at least in this file?
   
   I did that in the multi line viz that I'm working on. I noticed that on the 
multi deck.gl viz we're using jquery for async requests, but we can use 
`d3.json` instead, and get rid of the jquery dependency in those files.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] betodealmeida commented on a change in pull request #4800: Improve the calendar heatmap

2018-04-11 Thread GitBox
betodealmeida commented on a change in pull request #4800: Improve the calendar 
heatmap
URL: 
https://github.com/apache/incubator-superset/pull/4800#discussion_r180899647
 
 

 ##
 File path: superset/assets/javascripts/explore/stores/controls.jsx
 ##
 @@ -1003,6 +1010,46 @@ export const controls = {
 'relative to the time granularity selected'),
   },
 
+  cell_size: {
+type: 'TextControl',
+isInt: true,
+default: 10,
+validators: [v.integer],
+renderTrigger: true,
+label: t('Cell Size'),
+description: t('The size of the square cell, in pixels'),
+  },
+
+  cell_padding: {
+type: 'TextControl',
+isInt: true,
+validators: [v.integer],
+renderTrigger: true,
+default: 2,
+label: t('Cell Padding'),
+description: t('The distance between cells, in pixels'),
+  },
+
+  cell_radius: {
+type: 'TextControl',
+isInt: true,
+validators: [v.integer],
+renderTrigger: true,
+default: 2,
 
 Review comment:
   Personally, I'd leave this at zero — the round borders look really ugly to 
me, unless they're full circles. Maybe someone with a better design eye than me 
can comment on this.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] betodealmeida commented on a change in pull request #4800: Improve the calendar heatmap

2018-04-11 Thread GitBox
betodealmeida commented on a change in pull request #4800: Improve the calendar 
heatmap
URL: 
https://github.com/apache/incubator-superset/pull/4800#discussion_r180899636
 
 

 ##
 File path: superset/assets/javascripts/modules/colors.js
 ##
 @@ -122,6 +122,42 @@ export const spectrums = {
 '#FAFAFA',
 '#66',
   ],
+  greens: [
+'#cc',
+'#78c679',
+'#006837',
+  ],
+  purples: [
+'#f2f0f7',
+'#9e9ac8',
+'#54278f',
+  ],
+  oranges: [
+'#fef0d9',
+'#fc8d59',
+'#b3',
+  ],
+  red_yellow_blue: [
+'#d7191c',
+'#fdae61',
+'#bf',
+'#abd9e9',
+'#2c7bb6',
+  ],
+  brown_white_green: [
+'#a6611a',
+'#dfc27d',
+'#f5f5f5',
+'#80cdc1',
+'#018571',
+  ],
+  purple_white_green: [
 
 Review comment:
   I've seen some functions in Python (matplotlib perhaps?) where you can 
interpolate between colors. It would be cool to have something similar here, if 
it exists in Javascript (something [like 
this](https://www.npmjs.com/package/color-interpolate)). Imagine:
   
   ```javascript
   const brown = '#a6611a';
   const white = '#f5f5f5';
   const green = '#018571';
   ...
   const brown_white_green: interpolate([brown, white, green], 5),
   ...
   ```


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] betodealmeida commented on a change in pull request #4800: Improve the calendar heatmap

2018-04-11 Thread GitBox
betodealmeida commented on a change in pull request #4800: Improve the calendar 
heatmap
URL: 
https://github.com/apache/incubator-superset/pull/4800#discussion_r180901321
 
 

 ##
 File path: 
superset/migrations/versions/bf706ae5eb46_cal_heatmap_metric_to_metrics.py
 ##
 @@ -0,0 +1,56 @@
+"""cal_heatmap_metric_to_metrics
+
+Revision ID: bf706ae5eb46
+Revises: f231d82b9b26
+Create Date: 2018-04-10 11:19:47.621878
+
+"""
+from alembic import op
+
+import json
+from sqlalchemy.ext.declarative import declarative_base
+from sqlalchemy import Column, Integer, String, Text
+
+from superset import db
+from superset.legacy import cast_form_data
+
+Base = declarative_base()
+
+# revision identifiers, used by Alembic.
+revision = 'bf706ae5eb46'
+down_revision = 'f231d82b9b26'
+
+
+class Slice(Base):
+"""Declarative class to do query in upgrade"""
+__tablename__ = 'slices'
+id = Column(Integer, primary_key=True)
+datasource_type = Column(String(200))
+viz_type = Column(String(200))
+slice_name = Column(String(200))
+params = Column(Text)
+
+
+def upgrade():
+bind = op.get_bind()
+session = db.Session(bind=bind)
+
+slices = session.query(Slice).filter_by(viz_type='cal_heatmap').all()
+slice_len = len(slices)
+for i, slc in enumerate(slices):
+try:
+params = json.loads(slc.params or '{}')
+params['metrics'] = [params.get('metric')]
+del params['metric']
 
 Review comment:
   Nitty-nit, you can do this in one line:
   
   ```python
   params['metrics'] = [params.pop('metric')]
   ```


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services