From f7fc31cadef17cf9b350f26aede5e3baa068fce8 Mon Sep 17 00:00:00 2001 From: archmoj Date: Tue, 19 Oct 2021 13:22:30 -0400 Subject: [PATCH 1/4] fix smith chart react --- src/plots/smith/layout_attributes.js | 6 ++---- src/plots/smith/layout_defaults.js | 12 +----------- test/image/mocks/smith_basic.json | 3 +++ test/jasmine/tests/smith_test.js | 6 +++--- test/plot-schema.json | 15 ++++++++++++++- 5 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/plots/smith/layout_attributes.js b/src/plots/smith/layout_attributes.js index b6ac6c25c3f..225ac72f36d 100644 --- a/src/plots/smith/layout_attributes.js +++ b/src/plots/smith/layout_attributes.js @@ -73,12 +73,10 @@ var imaginaryAxisAttrs = extendFlat({ visible: extendFlat({}, axesAttrs.visible, {dflt: true}), tickvals: { + dflt: [-5, -2, -1, -0.5, -0.2, 0, 0.2, 0.5, 1, 2, 5], valType: 'data_array', editType: 'plot', - description: [ - 'Sets the values at which ticks on this axis appear.', - 'Defaults to `realaxis.tickvals` plus the same as negatives and zero.' - ].join(' ') + description: 'Sets the values at which ticks on this axis appear.' }, ticks: axesAttrs.ticks, diff --git a/src/plots/smith/layout_defaults.js b/src/plots/smith/layout_defaults.js index fc332959a41..c8c53a77c20 100644 --- a/src/plots/smith/layout_defaults.js +++ b/src/plots/smith/layout_defaults.js @@ -52,17 +52,7 @@ function handleDefaults(contIn, contOut, coerce, opts) { var isRealAxis = axName === 'realaxis'; if(isRealAxis) coerceAxis('side'); - if(isRealAxis) { - coerceAxis('tickvals'); - } else { - var realTickvals = contOut.realaxis.tickvals || layoutAttributes.realaxis.tickvals.dflt; - var imagTickvalsDflt = - realTickvals.slice().reverse().map(function(x) { return -x; }) - .concat([0]) - .concat(realTickvals); - - coerceAxis('tickvals', imagTickvalsDflt); - } + coerceAxis('tickvals'); var dfltColor; var dfltFontColor; diff --git a/test/image/mocks/smith_basic.json b/test/image/mocks/smith_basic.json index 463a03a83ca..93c91a3fc5d 100644 --- a/test/image/mocks/smith_basic.json +++ b/test/image/mocks/smith_basic.json @@ -25,6 +25,9 @@ "smith": { "realaxis": { "tickvals": [0.2, 0.4, 0.6, 0.8, 1, 1.5, 2, 3, 4, 5, 10, 20] + }, + "imaginaryaxis": { + "tickvals": [-20, -10, -5, -4, -3, -2, -1.5, -1, -0.8, -0.6, -0.4, -0.2, 0, 0.2, 0.4, 0.6, 0.8, 1, 1.5, 2, 3, 4, 5, 10, 20] } } } diff --git a/test/jasmine/tests/smith_test.js b/test/jasmine/tests/smith_test.js index cf655b30844..18127b1b086 100644 --- a/test/jasmine/tests/smith_test.js +++ b/test/jasmine/tests/smith_test.js @@ -106,11 +106,11 @@ describe('Test relayout on smith subplots:', function() { return Plotly.relayout(gd, 'smith.imaginaryaxis', {ticks: 'inside'}); }) .then(function() { - check(26, 'M-0.5,0h-5'); + check(12, 'M-0.5,0h-5'); return Plotly.relayout(gd, 'smith.imaginaryaxis', {ticks: 'outside'}); }) .then(function() { - check(26, 'M0.5,0h5'); + check(12, 'M0.5,0h5'); return Plotly.relayout(gd, 'smith.imaginaryaxis', {ticks: ''}); }) .then(function() { @@ -118,7 +118,7 @@ describe('Test relayout on smith subplots:', function() { return Plotly.relayout(gd, 'smith.imaginaryaxis', {ticks: 'inside'}); }) .then(function() { - check(26, 'M-0.5,0h-5'); + check(12, 'M-0.5,0h-5'); }) .then(done, done.fail); }); diff --git a/test/plot-schema.json b/test/plot-schema.json index a49f77f14a5..c86d9485b23 100644 --- a/test/plot-schema.json +++ b/test/plot-schema.json @@ -7510,7 +7510,20 @@ "valType": "string" }, "tickvals": { - "description": "Sets the values at which ticks on this axis appear. Defaults to `realaxis.tickvals` plus the same as negatives and zero.", + "description": "Sets the values at which ticks on this axis appear.", + "dflt": [ + -5, + -2, + -1, + -0.5, + -0.2, + 0, + 0.2, + 0.5, + 1, + 2, + 5 + ], "editType": "plot", "valType": "data_array" }, From 8fe37f4a03d8ac64035884d94d9e31a7198a0615 Mon Sep 17 00:00:00 2001 From: archmoj Date: Tue, 19 Oct 2021 13:53:36 -0400 Subject: [PATCH 2/4] update draft log for smith charts to include PR 5992 --- draftlogs/5956_add.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/draftlogs/5956_add.md b/draftlogs/5956_add.md index c790a7cb13f..7382acb7659 100644 --- a/draftlogs/5956_add.md +++ b/draftlogs/5956_add.md @@ -1,2 +1,2 @@ - - Add `smith` subplots and the `scattersmith` trace type for displaying Smith charts [[#5956](https://github.com/plotly/plotly.js/pull/5956)], + - Add `smith` subplots and the `scattersmith` trace type for displaying Smith charts [[#5956](https://github.com/plotly/plotly.js/pull/5956), [#5992](https://github.com/plotly/plotly.js/pull/5992)], with thanks to Kitware and @waxlamp for kicking off this effort. From 3380c0d85a9b3a44cc0b4ccebdb44126d0255b36 Mon Sep 17 00:00:00 2001 From: archmoj Date: Fri, 29 Oct 2021 13:46:04 -0400 Subject: [PATCH 3/4] Revert "fix smith chart react" This reverts commit f7fc31cadef17cf9b350f26aede5e3baa068fce8. --- src/plots/smith/layout_attributes.js | 6 ++++-- src/plots/smith/layout_defaults.js | 12 +++++++++++- test/image/mocks/smith_basic.json | 3 --- test/jasmine/tests/smith_test.js | 6 +++--- test/plot-schema.json | 15 +-------------- 5 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/plots/smith/layout_attributes.js b/src/plots/smith/layout_attributes.js index 225ac72f36d..b6ac6c25c3f 100644 --- a/src/plots/smith/layout_attributes.js +++ b/src/plots/smith/layout_attributes.js @@ -73,10 +73,12 @@ var imaginaryAxisAttrs = extendFlat({ visible: extendFlat({}, axesAttrs.visible, {dflt: true}), tickvals: { - dflt: [-5, -2, -1, -0.5, -0.2, 0, 0.2, 0.5, 1, 2, 5], valType: 'data_array', editType: 'plot', - description: 'Sets the values at which ticks on this axis appear.' + description: [ + 'Sets the values at which ticks on this axis appear.', + 'Defaults to `realaxis.tickvals` plus the same as negatives and zero.' + ].join(' ') }, ticks: axesAttrs.ticks, diff --git a/src/plots/smith/layout_defaults.js b/src/plots/smith/layout_defaults.js index c8c53a77c20..fc332959a41 100644 --- a/src/plots/smith/layout_defaults.js +++ b/src/plots/smith/layout_defaults.js @@ -52,7 +52,17 @@ function handleDefaults(contIn, contOut, coerce, opts) { var isRealAxis = axName === 'realaxis'; if(isRealAxis) coerceAxis('side'); - coerceAxis('tickvals'); + if(isRealAxis) { + coerceAxis('tickvals'); + } else { + var realTickvals = contOut.realaxis.tickvals || layoutAttributes.realaxis.tickvals.dflt; + var imagTickvalsDflt = + realTickvals.slice().reverse().map(function(x) { return -x; }) + .concat([0]) + .concat(realTickvals); + + coerceAxis('tickvals', imagTickvalsDflt); + } var dfltColor; var dfltFontColor; diff --git a/test/image/mocks/smith_basic.json b/test/image/mocks/smith_basic.json index 93c91a3fc5d..463a03a83ca 100644 --- a/test/image/mocks/smith_basic.json +++ b/test/image/mocks/smith_basic.json @@ -25,9 +25,6 @@ "smith": { "realaxis": { "tickvals": [0.2, 0.4, 0.6, 0.8, 1, 1.5, 2, 3, 4, 5, 10, 20] - }, - "imaginaryaxis": { - "tickvals": [-20, -10, -5, -4, -3, -2, -1.5, -1, -0.8, -0.6, -0.4, -0.2, 0, 0.2, 0.4, 0.6, 0.8, 1, 1.5, 2, 3, 4, 5, 10, 20] } } } diff --git a/test/jasmine/tests/smith_test.js b/test/jasmine/tests/smith_test.js index 18127b1b086..cf655b30844 100644 --- a/test/jasmine/tests/smith_test.js +++ b/test/jasmine/tests/smith_test.js @@ -106,11 +106,11 @@ describe('Test relayout on smith subplots:', function() { return Plotly.relayout(gd, 'smith.imaginaryaxis', {ticks: 'inside'}); }) .then(function() { - check(12, 'M-0.5,0h-5'); + check(26, 'M-0.5,0h-5'); return Plotly.relayout(gd, 'smith.imaginaryaxis', {ticks: 'outside'}); }) .then(function() { - check(12, 'M0.5,0h5'); + check(26, 'M0.5,0h5'); return Plotly.relayout(gd, 'smith.imaginaryaxis', {ticks: ''}); }) .then(function() { @@ -118,7 +118,7 @@ describe('Test relayout on smith subplots:', function() { return Plotly.relayout(gd, 'smith.imaginaryaxis', {ticks: 'inside'}); }) .then(function() { - check(12, 'M-0.5,0h-5'); + check(26, 'M-0.5,0h-5'); }) .then(done, done.fail); }); diff --git a/test/plot-schema.json b/test/plot-schema.json index c86d9485b23..a49f77f14a5 100644 --- a/test/plot-schema.json +++ b/test/plot-schema.json @@ -7510,20 +7510,7 @@ "valType": "string" }, "tickvals": { - "description": "Sets the values at which ticks on this axis appear.", - "dflt": [ - -5, - -2, - -1, - -0.5, - -0.2, - 0, - 0.2, - 0.5, - 1, - 2, - 5 - ], + "description": "Sets the values at which ticks on this axis appear. Defaults to `realaxis.tickvals` plus the same as negatives and zero.", "editType": "plot", "valType": "data_array" }, From 844e24c6b070a907305bfb8dfdd76518986e08fc Mon Sep 17 00:00:00 2001 From: archmoj Date: Fri, 29 Oct 2021 15:30:40 -0400 Subject: [PATCH 4/4] AJ's suggestion to memorize imaginary tickval defaults --- src/plots/smith/layout_defaults.js | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/src/plots/smith/layout_defaults.js b/src/plots/smith/layout_defaults.js index fc332959a41..1a36a46dd59 100644 --- a/src/plots/smith/layout_defaults.js +++ b/src/plots/smith/layout_defaults.js @@ -16,6 +16,12 @@ var layoutAttributes = require('./layout_attributes'); var constants = require('./constants'); var axisNames = constants.axisNames; +var makeImagDflt = memoize(function(realTickvals) { + return realTickvals.slice().reverse().map(function(x) { return -x; }) + .concat([0]) + .concat(realTickvals); +}, String); + function handleDefaults(contIn, contOut, coerce, opts) { var bgColor = coerce('bgcolor'); opts.bgColor = Color.combine(bgColor, opts.paper_bgcolor); @@ -55,11 +61,10 @@ function handleDefaults(contIn, contOut, coerce, opts) { if(isRealAxis) { coerceAxis('tickvals'); } else { - var realTickvals = contOut.realaxis.tickvals || layoutAttributes.realaxis.tickvals.dflt; - var imagTickvalsDflt = - realTickvals.slice().reverse().map(function(x) { return -x; }) - .concat([0]) - .concat(realTickvals); + var imagTickvalsDflt = makeImagDflt( + contOut.realaxis.tickvals || + layoutAttributes.realaxis.tickvals.dflt + ); coerceAxis('tickvals', imagTickvalsDflt); } @@ -132,3 +137,15 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) { layoutOut: layoutOut }); }; + +function memoize(fn, keyFn) { + var cache = {}; + return function(val) { + var newKey = keyFn ? keyFn(val) : val; + if(newKey in cache) { return cache[newKey]; } + + var out = fn(val); + cache[newKey] = out; + return out; + }; +}