Merge pull request #3023 from plotly/barpolar-validate

Fix #3022 - validate + polar.bargap

Author: etpinard

src/plot_api/validate.js

@@ -281,16 +281,23 @@ function crawl(objIn, objOut, schema, list, base, path) {
 
 // the 'full' layout schema depends on the traces types presents
 function fillLayoutSchema(schema, dataOut) {
+    var layoutSchema = schema.layout.layoutAttributes;
+
     for(var i = 0; i < dataOut.length; i++) {
-        var traceType = dataOut[i].type,
-            traceLayoutAttr = schema.traces[traceType].layoutAttributes;
+        var traceOut = dataOut[i];
+        var traceSchema = schema.traces[traceOut.type];
+        var traceLayoutAttr = traceSchema.layoutAttributes;
 
         if(traceLayoutAttr) {
-            Lib.extendFlat(schema.layout.layoutAttributes, traceLayoutAttr);
+            if(traceOut.subplot) {
+                Lib.extendFlat(layoutSchema[traceSchema.attributes.subplot.dflt], traceLayoutAttr);
+            } else {
+                Lib.extendFlat(layoutSchema, traceLayoutAttr);
+            }
         }
     }
 
-    return schema.layout.layoutAttributes;
+    return layoutSchema;
 }
 
 // validation error codes

src/plots/polar/legacy/area_attributes.js

@@ -10,15 +10,53 @@
 
 var scatterAttrs = require('../../../traces/scatter/attributes');
 var scatterMarkerAttrs = scatterAttrs.marker;
+var extendFlat = require('../../../lib/extend').extendFlat;
+
+var deprecationWarning = [
+    'Area traces are deprecated!',
+    'Please switch to the *barpolar* trace type.'
+].join(' ');
 
 module.exports = {
-    r: scatterAttrs.r,
-    t: scatterAttrs.t,
+    r: extendFlat({}, scatterAttrs.r, {
+        description: [
+            deprecationWarning,
+            'Sets the radial coordinates',
+            'for legacy polar chart only.'
+        ].join(' ')
+    }),
+    t: extendFlat({}, scatterAttrs.t, {
+        description: [
+            deprecationWarning,
+            'Sets the angular coordinates',
+            'for legacy polar chart only.'
+        ].join(' ')
+    }),
     marker: {
-        color: scatterMarkerAttrs.color,
-        size: scatterMarkerAttrs.size,
-        symbol: scatterMarkerAttrs.symbol,
-        opacity: scatterMarkerAttrs.opacity,
+        color: extendFlat({}, scatterMarkerAttrs.color, {
+            description: [
+                deprecationWarning,
+                scatterMarkerAttrs.color.description
+            ].join(' ')
+        }),
+        size: extendFlat({}, scatterMarkerAttrs.size, {
+            description: [
+                deprecationWarning,
+                scatterMarkerAttrs.size.description
+            ].join(' ')
+        }),
+        symbol: extendFlat({}, scatterMarkerAttrs.symbol, {
+            description: [
+                deprecationWarning,
+                scatterMarkerAttrs.symbol.description
+            ].join(' ')
+        }),
+        opacity: extendFlat({}, scatterMarkerAttrs.opacity, {
+            description: [
+                deprecationWarning,
+                scatterMarkerAttrs.opacity.description
+            ].join(' ')
+        }),
         editType: 'calc'
     }
 };

src/plots/polar/legacy/axis_attributes.js

@@ -13,6 +13,11 @@ var axesAttrs = require('../../cartesian/layout_attributes');
 var extendFlat = require('../../../lib/extend').extendFlat;
 var overrideAll = require('../../../plot_api/edit_types').overrideAll;
 
+var deprecationWarning = [
+    'Legacy polar charts are deprecated!',
+    'Please switch to *polar* subplots.'
+].join(' ');
+
 var domainAttr = extendFlat({}, axesAttrs.domain, {
     description: [
         'Polar chart subplots are not supported yet.',
@@ -26,6 +31,7 @@ function mergeAttrs(axisName, nonCommonAttrs) {
             valType: 'boolean',
             role: 'style',
             description: [
+                deprecationWarning,
                 'Determines whether or not the line bounding this',
                 axisName, 'axis',
                 'will be shown on the figure.'
@@ -35,6 +41,7 @@ function mergeAttrs(axisName, nonCommonAttrs) {
             valType: 'boolean',
             role: 'style',
             description: [
+                deprecationWarning,
                 'Determines whether or not the',
                 axisName, 'axis ticks',
                 'will feature tick labels.'
@@ -45,6 +52,7 @@ function mergeAttrs(axisName, nonCommonAttrs) {
             values: ['horizontal', 'vertical'],
             role: 'style',
             description: [
+                deprecationWarning,
                 'Sets the orientation (from the paper perspective)',
                 'of the', axisName, 'axis tick labels.'
             ].join(' ')
@@ -54,31 +62,36 @@ function mergeAttrs(axisName, nonCommonAttrs) {
             min: 0,
             role: 'style',
             description: [
+                deprecationWarning,
                 'Sets the length of the tick lines on this', axisName, 'axis.'
             ].join(' ')
         },
         tickcolor: {
             valType: 'color',
             role: 'style',
             description: [
+                deprecationWarning,
                 'Sets the color of the tick lines on this', axisName, 'axis.'
             ].join(' ')
         },
         ticksuffix: {
             valType: 'string',
             role: 'style',
             description: [
+                deprecationWarning,
                 'Sets the length of the tick lines on this', axisName, 'axis.'
             ].join(' ')
         },
         endpadding: {
             valType: 'number',
-            role: 'style'
+            role: 'style',
+            description: deprecationWarning,
         },
         visible: {
             valType: 'boolean',
             role: 'info',
             description: [
+                deprecationWarning,
                 'Determines whether or not this axis will be visible.'
             ].join(' ')
         }
@@ -97,6 +110,7 @@ module.exports = overrideAll({
                 { valType: 'number' }
             ],
             description: [
+                deprecationWarning,
                 'Defines the start and end point of this radial axis.'
             ].join(' ')
         },
@@ -105,6 +119,7 @@ module.exports = overrideAll({
             valType: 'number',
             role: 'style',
             description: [
+                deprecationWarning,
                 'Sets the orientation (an angle with respect to the origin)',
                 'of the radial axis.'
             ].join(' ')
@@ -120,6 +135,7 @@ module.exports = overrideAll({
                 { valType: 'number', dflt: 360 }
             ],
             description: [
+                deprecationWarning,
                 'Defines the start and end point of this angular axis.'
             ].join(' ')
         },
@@ -133,16 +149,18 @@ module.exports = overrideAll({
             values: ['clockwise', 'counterclockwise'],
             role: 'info',
             description: [
-                'For polar plots only.',
-                'Sets the direction corresponding to positive angles.'
+                deprecationWarning,
+                'Sets the direction corresponding to positive angles',
+                'in legacy polar charts.'
             ].join(' ')
         },
         orientation: {
             valType: 'angle',
             role: 'info',
             description: [
-                'For polar plots only.',
-                'Rotates the entire polar by the given angle.'
+                deprecationWarning,
+                'Rotates the entire polar by the given angle',
+                'in legacy polar charts.'
             ].join(' ')
         }
     }

src/traces/barpolar/layout_defaults.js

@@ -11,17 +11,23 @@
 var Lib = require('../../lib');
 var attrs = require('./layout_attributes');
 
-module.exports = function(layoutIn, layoutOut) {
-    var subplotIds = layoutOut._subplots.polar;
+module.exports = function(layoutIn, layoutOut, fullData) {
+    var subplotsDone = {};
     var sp;
 
     function coerce(attr, dflt) {
         return Lib.coerce(layoutIn[sp] || {}, layoutOut[sp], attrs, attr, dflt);
     }
 
-    for(var i = 0; i < subplotIds.length; i++) {
-        sp = subplotIds[i];
-        coerce('barmode');
-        coerce('bargap');
+    for(var i = 0; i < fullData.length; i++) {
+        var trace = fullData[i];
+        if(trace.type === 'barpolar' && trace.visible === true) {
+            sp = trace.subplot;
+            if(!subplotsDone[sp]) {
+                coerce('barmode');
+                coerce('bargap');
+                subplotsDone[sp] = 1;
+            }
+        }
     }
 };

src/traces/scatter/attributes.js

@@ -544,18 +544,20 @@ module.exports = {
         valType: 'data_array',
         editType: 'calc',
         description: [
-            'For legacy polar chart only.',
-            'Please switch to *scatterpolar* trace type.',
-            'Sets the radial coordinates.'
+            'r coordinates in scatter traces are deprecated!',
+            'Please switch to the *scatterpolar* trace type.',
+            'Sets the radial coordinates',
+            'for legacy polar chart only.'
         ].join('')
     },
     t: {
         valType: 'data_array',
         editType: 'calc',
         description: [
-            'For legacy polar chart only.',
-            'Please switch to *scatterpolar* trace type.',
-            'Sets the angular coordinates.'
+            't coordinates in scatter traces are deprecated!',
+            'Please switch to the *scatterpolar* trace type.',
+            'Sets the angular coordinates',
+            'for legacy polar chart only.'
         ].join('')
     }
 };

test/jasmine/tests/barpolar_test.js

@@ -1,10 +1,43 @@
 var Plotly = require('@lib');
 var Lib = require('@src/lib');
 
+var supplyAllDefaults = require('../assets/supply_defaults');
 var createGraphDiv = require('../assets/create_graph_div');
 var destroyGraphDiv = require('../assets/destroy_graph_div');
 var failTest = require('../assets/fail_test');
 
+describe('Test barpolar defaults:', function() {
+    var gd;
+
+    function _supply(opts, layout) {
+        gd = {};
+        opts = Array.isArray(opts) ? opts : [opts];
+
+        gd.data = opts.map(function(o) {
+            return Lib.extendFlat({type: 'barpolar'}, o || {});
+        });
+        gd.layout = layout || {};
+
+        supplyAllDefaults(gd);
+    }
+
+    it('should not coerce polar.bar* attributes on subplot w/o visible barpolar', function() {
+        _supply([
+            {visible: false, subplot: 'polar'},
+            {r: [1], subplot: 'polar2'},
+            {type: 'scatterpolar', r: [1], subplot: 'polar3'}
+        ]);
+
+        var fullLayout = gd._fullLayout;
+        expect(fullLayout.polar.barmode).toBeUndefined();
+        expect(fullLayout.polar.bargap).toBeUndefined();
+        expect(fullLayout.polar2.barmode).toBe('stack');
+        expect(fullLayout.polar2.bargap).toBe(0.1);
+        expect(fullLayout.polar3.barmode).toBeUndefined();
+        expect(fullLayout.polar3.bargap).toBeUndefined();
+    });
+});
+
 describe('Test barpolar hover:', function() {
     var gd;
 

test/jasmine/tests/validate_test.js

@@ -539,4 +539,33 @@ describe('Plotly.validate', function() {
         var out = Plotly.validate(shapeMock.data, shapeMock.layout);
         expect(out).toBeUndefined();
     });
+
+    it('should work with *trace* layout attributes', function() {
+        var out = Plotly.validate([{
+            type: 'bar',
+            y: [1, 2, 1]
+        }, {
+            type: 'barpolar',
+            r: [1, 2, 3]
+        }, {
+            type: 'scatterpolar',
+            theta: [0, 90, 200],
+            subplot: 'polar2'
+        }], {
+            bargap: 0.3,
+            polar: {bargap: 0.2},
+            polar2: {bargap: 0.05},
+            polar3: {bargap: 0.4}
+        });
+
+        expect(out.length).toBe(2);
+        assertErrorContent(
+            out[0], 'unused', 'layout', null, ['polar2', 'bargap'], 'polar2.bargap',
+            'In layout, key polar2.bargap did not get coerced'
+        );
+        assertErrorContent(
+            out[1], 'unused', 'layout', null, ['polar3'], 'polar3',
+            'In layout, container polar3 did not get coerced'
+        );
+    });
 });