[GitHub] betodealmeida commented on a change in pull request #4800: Improve the calendar heatmap
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
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
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
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
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
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