Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #3022 - validate + polar.bargap #3023

Merged
merged 5 commits into from
Sep 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions src/plot_api/validate.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
50 changes: 44 additions & 6 deletions src/plots/polar/legacy/area_attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
alexcjohnson marked this conversation as resolved.
Show resolved Hide resolved
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'
}
};
28 changes: 23 additions & 5 deletions src/plots/polar/legacy/axis_attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.',
Expand All @@ -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.'
Expand All @@ -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.'
Expand All @@ -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(' ')
Expand All @@ -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(' ')
}
Expand All @@ -97,6 +110,7 @@ module.exports = overrideAll({
{ valType: 'number' }
],
description: [
deprecationWarning,
'Defines the start and end point of this radial axis.'
].join(' ')
},
Expand All @@ -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(' ')
Expand All @@ -120,6 +135,7 @@ module.exports = overrideAll({
{ valType: 'number', dflt: 360 }
],
description: [
deprecationWarning,
'Defines the start and end point of this angular axis.'
].join(' ')
},
Expand All @@ -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(' ')
}
}
Expand Down
18 changes: 12 additions & 6 deletions src/traces/barpolar/layout_defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}
};
14 changes: 8 additions & 6 deletions src/traces/scatter/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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('')
}
};
33 changes: 33 additions & 0 deletions test/jasmine/tests/barpolar_test.js
Original file line number Diff line number Diff line change
@@ -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;

Expand Down
29 changes: 29 additions & 0 deletions test/jasmine/tests/validate_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
);
});
});