You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@superset.apache.org by GitBox <gi...@apache.org> on 2018/04/20 22:55:27 UTC

[GitHub] williaster closed pull request #4843: [formats] add better defaults for time + number formatting

williaster closed pull request #4843: [formats] add better defaults for time + number formatting
URL: https://github.com/apache/incubator-superset/pull/4843
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/superset/assets/spec/javascripts/modules/dates_spec.js b/superset/assets/spec/javascripts/modules/dates_spec.js
index 94dbfa6b1b..7101b1dad2 100644
--- a/superset/assets/spec/javascripts/modules/dates_spec.js
+++ b/superset/assets/spec/javascripts/modules/dates_spec.js
@@ -3,6 +3,7 @@ import { expect } from 'chai';
 import {
   tickMultiFormat,
   formatDate,
+  formatDateVerbose,
   fDuration,
   now,
   epochTimeXHoursAgo,
@@ -25,13 +26,35 @@ describe('formatDate', () => {
     expect(formatDate(new Date('2020-01-01'))).to.equal('2020');
   });
 
+  it('shows only month when 1st of month', () => {
+    expect(formatDate(new Date('2020-03-01'))).to.equal('March');
+  });
+
+  it('does not show day of week when it is Sunday', () => {
+    expect(formatDate(new Date('2020-03-15'))).to.equal('Mar 15');
+  });
+
+  it('shows weekday when it is not Sunday (and no ms/sec/min/hr)', () => {
+    expect(formatDate(new Date('2020-03-03'))).to.equal('Tue 03');
+  });
+});
+
+describe('formatDateVerbose', () => {
+  it('is a function', () => {
+    assert.isFunction(formatDateVerbose);
+  });
+
+  it('shows only year when 1st day of the year', () => {
+    expect(formatDateVerbose(new Date('2020-01-01'))).to.equal('2020');
+  });
+
   it('shows month and year when 1st of month', () => {
-    expect(formatDate(new Date('2020-03-01'))).to.equal('Mar 2020');
+    expect(formatDateVerbose(new Date('2020-03-01'))).to.equal('Mar 2020');
   });
 
   it('shows weekday when any day of the month', () => {
-    expect(formatDate(new Date('2020-03-03'))).to.equal('Tue Mar 3');
-    expect(formatDate(new Date('2020-03-15'))).to.equal('Sun Mar 15');
+    expect(formatDateVerbose(new Date('2020-03-03'))).to.equal('Tue Mar 3');
+    expect(formatDateVerbose(new Date('2020-03-15'))).to.equal('Sun Mar 15');
   });
 });
 
diff --git a/superset/assets/spec/javascripts/modules/utils_spec.jsx b/superset/assets/spec/javascripts/modules/utils_spec.jsx
index f219c6e706..4a319afbe1 100644
--- a/superset/assets/spec/javascripts/modules/utils_spec.jsx
+++ b/superset/assets/spec/javascripts/modules/utils_spec.jsx
@@ -1,8 +1,13 @@
 import { it, describe } from 'mocha';
 import { expect } from 'chai';
 import {
-  tryNumify, slugify, formatSelectOptionsForRange, d3format,
-  d3FormatPreset, d3TimeFormatPreset, defaultNumberFormatter,
+  tryNumify,
+  slugify,
+  formatSelectOptionsForRange,
+  d3format,
+  d3FormatPreset,
+  d3TimeFormatPreset,
+  defaultNumberFormatter,
   mainMetric,
 } from '../../../src/modules/utils';
 
@@ -53,12 +58,13 @@ describe('utils', () => {
       expect(d3FormatPreset('smart_date')(0)).to.equal('1970');
     });
   });
-  describe('d3TimeFormatPreset', () => {
+  describe('defaultNumberFormatter', () => {
     expect(defaultNumberFormatter(10)).to.equal('10');
     expect(defaultNumberFormatter(1)).to.equal('1');
     expect(defaultNumberFormatter(1.0)).to.equal('1');
     expect(defaultNumberFormatter(10.0)).to.equal('10');
     expect(defaultNumberFormatter(10001)).to.equal('10.0k');
+    expect(defaultNumberFormatter(10100)).to.equal('10.1k');
     expect(defaultNumberFormatter(111000000)).to.equal('111M');
     expect(defaultNumberFormatter(0.23)).to.equal('230m');
 
@@ -67,6 +73,7 @@ describe('utils', () => {
     expect(defaultNumberFormatter(-1.0)).to.equal('-1');
     expect(defaultNumberFormatter(-10.0)).to.equal('-10');
     expect(defaultNumberFormatter(-10001)).to.equal('-10.0k');
+    expect(defaultNumberFormatter(-10101)).to.equal('-10.1k');
     expect(defaultNumberFormatter(-111000000)).to.equal('-111M');
     expect(defaultNumberFormatter(-0.23)).to.equal('-230m');
   });
diff --git a/superset/assets/src/explore/stores/controls.jsx b/superset/assets/src/explore/stores/controls.jsx
index 0a6443318e..2dc7cf205f 100644
--- a/superset/assets/src/explore/stores/controls.jsx
+++ b/superset/assets/src/explore/stores/controls.jsx
@@ -56,14 +56,14 @@ const D3_FORMAT_DOCS = 'D3 format syntax: https://github.com/d3/d3-format';
 
 // input choices & options
 const D3_FORMAT_OPTIONS = [
-  ['.1s', '.1s | 12k'],
-  ['.3s', '.3s | 12.3k'],
-  ['.1%', '.1% | 12.3%'],
-  ['.3%', '.3% | 1234543.210%'],
-  ['.4r', '.4r | 12350'],
-  ['.3f', '.3f | 12345.432'],
-  ['+,', '+, | +12,345.4321'],
-  ['$,.2f', '$,.2f | $12,345.43'],
+  ['.1s', '.1s (12345.432 => 10k)'],
+  ['.3s', '.3s (12345.432 => 12.3k)'],
+  [',.1%', ',.1% (12345.432 => 1,234,543.2%)'],
+  ['.3%', '.3% (12345.432 => 1234543.200%)'],
+  ['.4r', '.4r (12345.432 => 12350)'],
+  [',.3f', ',.3f (12345.432 => 12,345.432)'],
+  ['+,', '+, (12345.432 => +12,345.432)'],
+  ['$,.2f', '$,.2f (12345.432 => $12,345.43)'],
 ];
 
 const ROW_LIMIT_OPTIONS = [10, 50, 100, 250, 500, 1000, 5000, 10000, 50000];
@@ -1537,7 +1537,7 @@ export const controls = {
     type: 'CheckboxControl',
     label: t('Rich Tooltip'),
     renderTrigger: true,
-    default: true,
+    default: false,
     description: t('The rich tooltip shows a list of all series for that ' +
     'point in time'),
   },
diff --git a/superset/assets/src/modules/dates.js b/superset/assets/src/modules/dates.js
index 91fde25171..f8d8684f73 100644
--- a/superset/assets/src/modules/dates.js
+++ b/superset/assets/src/modules/dates.js
@@ -12,7 +12,40 @@ export function UTC(dttm) {
     dttm.getUTCSeconds(),
   );
 }
-export const tickMultiFormat = d3.time.format.multi([
+
+export const tickMultiFormat = (() => {
+  const formatMillisecond = d3.time.format('.%Lms');
+  const formatSecond = d3.time.format(':%Ss');
+  const formatMinute = d3.time.format('%I:%M');
+  const formatHour = d3.time.format('%I %p');
+  const formatDay = d3.time.format('%a %d');
+  const formatWeek = d3.time.format('%b %d');
+  const formatMonth = d3.time.format('%B');
+  const formatYear = d3.time.format('%Y');
+
+  return function tickMultiFormatConcise(date) {
+    let formatter;
+    if (d3.time.second(date) < date) {
+      formatter = formatMillisecond;
+    } else if (d3.time.minute(date) < date) {
+      formatter = formatSecond;
+    } else if (d3.time.hour(date) < date) {
+      formatter = formatMinute;
+    } else if (d3.time.day(date) < date) {
+      formatter = formatHour;
+    } else if (d3.time.month(date) < date) {
+      formatter = d3.time.week(date) < date ? formatDay : formatWeek;
+    } else if (d3.time.year(date) < date) {
+      formatter = formatMonth;
+    } else {
+      formatter = formatYear;
+    }
+
+    return formatter(date);
+  };
+})();
+
+export const tickMultiFormatVerbose = d3.time.format.multi([
   [
     '.%L',
     function (d) {
@@ -74,6 +107,11 @@ export const formatDate = function (dttm) {
   return tickMultiFormat(d);
 };
 
+export const formatDateVerbose = function (dttm) {
+  const d = UTC(new Date(dttm));
+  return tickMultiFormatVerbose(d);
+};
+
 export const formatDateThunk = function (format) {
   if (!format) {
     return formatDate;
diff --git a/superset/assets/src/visualizations/nvd3_vis.js b/superset/assets/src/visualizations/nvd3_vis.js
index bf87287c78..4a3faba240 100644
--- a/superset/assets/src/visualizations/nvd3_vis.js
+++ b/superset/assets/src/visualizations/nvd3_vis.js
@@ -13,6 +13,7 @@ import AnnotationTypes, {
   applyNativeColumns,
 } from '../modules/AnnotationTypes';
 import { customizeToolTip, d3TimeFormatPreset, d3FormatPreset, tryNumify } from '../modules/utils';
+import { formatDateVerbose } from '../modules/dates';
 import { isTruthy } from '../utils/common';
 import { t } from '../locales';
 
@@ -136,7 +137,7 @@ export default function nvd3Vis(slice, payload) {
   };
 
   const vizType = fd.viz_type;
-  const f = d3.format('.3s');
+  const formatter = d3.format('.3s');
   const reduceXTicks = fd.reduce_x_ticks || false;
   let stacked = false;
   let row;
@@ -156,8 +157,6 @@ export default function nvd3Vis(slice, payload) {
     if (fd.x_ticks_layout === 'auto') {
       if (['column', 'dist_bar'].indexOf(vizType) >= 0) {
         xLabelRotation = 45;
-      } else if (isTimeSeries) {
-        staggerLabels = true;
       }
     } else if (fd.x_ticks_layout === 'staggered') {
       staggerLabels = true;
@@ -187,8 +186,6 @@ export default function nvd3Vis(slice, payload) {
         } else {
           chart = nv.models.lineChart();
         }
-        // To alter the tooltip header
-        // chart.interactiveLayer.tooltip.headerFormatter(function(){return '';});
         chart.xScale(d3.time.scale.utc());
         chart.interpolate(fd.line_interpolation);
         break;
@@ -303,9 +300,9 @@ export default function nvd3Vis(slice, payload) {
             `<tr><td style="color: ${p.color};">` +
               `<strong>${p[fd.entity]}</strong> (${p.group})` +
             '</td></tr>');
-          s += row(fd.x, f(p.x));
-          s += row(fd.y, f(p.y));
-          s += row(fd.size, f(p.size));
+          s += row(fd.x, formatter(p.x));
+          s += row(fd.y, formatter(p.y));
+          s += row(fd.size, formatter(p.size));
           s += '</table>';
           return s;
         });
@@ -375,6 +372,8 @@ export default function nvd3Vis(slice, payload) {
     let xAxisFormatter = d3FormatPreset(fd.x_axis_format);
     if (isTimeSeries) {
       xAxisFormatter = d3TimeFormatPreset(fd.x_axis_format);
+      // In tooltips, always use the verbose time format
+      chart.interactiveLayer.tooltip.headerFormatter(formatDateVerbose);
     }
     if (chart.x2Axis && chart.x2Axis.tickFormat) {
       chart.x2Axis.tickFormat(xAxisFormatter);
@@ -397,6 +396,13 @@ export default function nvd3Vis(slice, payload) {
       chart.y2Axis.tickFormat(yAxisFormatter);
     }
 
+    if (chart.yAxis) {
+      chart.yAxis.ticks(5);
+    }
+    if (chart.y2Axis) {
+      chart.y2Axis.ticks(5);
+    }
+
 
     // Set showMaxMin for all axis
     function setAxisShowMaxMin(axis, showminmax) {
@@ -404,10 +410,12 @@ export default function nvd3Vis(slice, payload) {
         axis.showMaxMin(showminmax);
       }
     }
-    setAxisShowMaxMin(chart.xAxis, fd.x_axis_showminmax);
-    setAxisShowMaxMin(chart.x2Axis, fd.x_axis_showminmax);
-    setAxisShowMaxMin(chart.yAxis, fd.y_axis_showminmax);
-    setAxisShowMaxMin(chart.y2Axis, fd.y_axis_showminmax);
+
+    // If these are undefined, they register as truthy
+    setAxisShowMaxMin(chart.xAxis, fd.x_axis_showminmax || false);
+    setAxisShowMaxMin(chart.x2Axis, fd.x_axis_showminmax || false);
+    setAxisShowMaxMin(chart.yAxis, fd.y_axis_showminmax || false);
+    setAxisShowMaxMin(chart.y2Axis, fd.y_axis_showminmax || false);
 
     if (vizType === 'time_pivot') {
       chart.color((d) => {
@@ -425,10 +433,11 @@ export default function nvd3Vis(slice, payload) {
       chart.useInteractiveGuideline(true);
       if (vizType === 'line') {
         // Custom sorted tooltip
+        // use a verbose formatter for times
         chart.interactiveLayer.tooltip.contentGenerator((d) => {
           let tooltip = '';
           tooltip += "<table><thead><tr><td colspan='3'>"
-            + `<strong class='x-value'>${xAxisFormatter(d.value)}</strong>`
+            + `<strong class='x-value'>${formatDateVerbose(d.value)}</strong>`
             + '</td></tr></thead><tbody>';
           d.series.sort((a, b) => a.value >= b.value ? -1 : 1);
           d.series.forEach((series) => {
diff --git a/superset/assets/src/visualizations/sunburst.js b/superset/assets/src/visualizations/sunburst.js
index 0c04622c60..0c9c2cc5ee 100644
--- a/superset/assets/src/visualizations/sunburst.js
+++ b/superset/assets/src/visualizations/sunburst.js
@@ -43,8 +43,8 @@ function sunburstVis(slice, payload) {
       return Math.sqrt(d.y + d.dy);
     });
 
-  const formatNum = d3.format('.3s');
-  const formatPerc = d3.format('.3p');
+  const formatNum = d3.format('.1s');
+  const formatPerc = d3.format('.1p');
 
   container.select('svg').remove();
 
diff --git a/superset/assets/src/visualizations/world_map.js b/superset/assets/src/visualizations/world_map.js
index a9ab714a5e..e7c1047ca7 100644
--- a/superset/assets/src/visualizations/world_map.js
+++ b/superset/assets/src/visualizations/world_map.js
@@ -39,7 +39,7 @@ function worldMapChart(slice, payload) {
     mapData[d.country] = d;
   });
 
-  const f = d3.format('.3s');
+  const formatter = d3.format('.3s');
 
   container.show();
 
@@ -58,7 +58,7 @@ function worldMapChart(slice, payload) {
       highlightFillColor: '#005a63',
       highlightBorderWidth: 1,
       popupTemplate: (geo, d) => (
-        `<div class="hoverinfo"><strong>${d.name}</strong><br>${f(d.m1)}</div>`
+        `<div class="hoverinfo"><strong>${d.name}</strong><br>${formatter(d.m1)}</div>`
       ),
     },
     bubblesConfig: {
@@ -68,7 +68,7 @@ function worldMapChart(slice, payload) {
       popupOnHover: true,
       radius: null,
       popupTemplate: (geo, d) => (
-        `<div class="hoverinfo"><strong>${d.name}</strong><br>${f(d.m2)}</div>`
+        `<div class="hoverinfo"><strong>${d.name}</strong><br>${formatter(d.m2)}</div>`
       ),
       fillOpacity: 0.5,
       animate: true,


 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services