diff --git a/src/firefly/js/charts/AxisRangeUtil.js b/src/firefly/js/charts/AxisRangeUtil.js new file mode 100644 index 0000000000..ebcd8a9422 --- /dev/null +++ b/src/firefly/js/charts/AxisRangeUtil.js @@ -0,0 +1,84 @@ +/* + * License information at https://github.com/Caltech-IPAC/firefly/blob/master/License.txt + */ + +/** + * Whether a plotly axis is reversed. + * Plotly keeps the direction as autorange 'reversed' or as the order of the range bounds (range[0] > range[1]). + * @param {object} axisLayout - plotly layout of the axis + * @returns {boolean} + */ +export function isAxisReversed(axisLayout) { + const autorange = axisLayout?.autorange; + const range = axisLayout?.range || []; + return (autorange === 'reversed') || (range[1] < range[0]); +} + +/** + * Get range for a plotly axis + * Plotly requires range to be reversed if the axis is reversed, + * and limits to be log if axis scale is log + * @param min - minimum value + * @param max - maximum value + * @param isLog - true, if an axis uses log scale + * @param isReversed - true, if the axis should be reversed + * @returns {Array} an array for axis range property in plotly layout + */ +export function getRange(min, max, isLog, isReversed) { + const [r1, r2] = isReversed ? [max, min] : [min, max]; + return isLog ? [Math.log10(r1), Math.log10(r2)] : [r1, r2]; +} + +/** + * Evaluate the plotly range changes of one axis from the chart options fields. + * + * Everything is decided from what the user asked for in the fields (reverse/log options, bounds, axis column change), + * with the current axis layout used only for the current range and for whatever the fields don't specify. + * + * @param {object} p + * @param {string} p.a - axis: 'x' or 'y' + * @param {object} p.fields - chart options fields (fieldKey -> value) + * @param {object} [p.axisLayout] - current plotly layout of the axis + * @returns {object} changes for `layout.axis.range` and `layout.axis.autorange`; + * empty when the axis range should be left as is + */ +export function evalAxisRangeChanges({a, fields, axisLayout={}}) { + const rangeKey = `layout.${a}axis.range`; + const autorangeKey = `layout.${a}axis.autorange`; + + const hasOpts = `__${a}options` in fields; + const opts = fields[`__${a}options`] || ''; + const isCurrLog = axisLayout.type === 'log'; + const flip = hasOpts ? opts.includes('flip') : isAxisReversed(axisLayout); + const isLog = hasOpts ? opts.includes('log') : isCurrLog; + const scaleChanged = isLog !== isCurrLog; // log range is in log10 units, so a switch needs a rebuild + const reset = Boolean(fields[`__${a}reset`]); + let min = parseFloat(fields[`fireflyLayout.${a}axis.min`]); + let max = parseFloat(fields[`fireflyLayout.${a}axis.max`]); + const hasBounds = !Number.isNaN(min) || !Number.isNaN(max); + + const currRange = !reset && axisLayout.range; // reset: axis column changed, current range doesn't apply + const autoscale = {[autorangeKey]: flip ? 'reversed' : true, [rangeKey]: undefined}; + + if (!hasBounds) { + if (!hasOpts) return {}; // caller doesn't manage this axis + if (!currRange) return autoscale; // nothing to keep + if (!scaleChanged) { // keep the current range, reordered to the requested direction + const [r0, r1] = currRange; + const needsSwap = flip ? r0 < r1 : r1 < r0; + return needsSwap ? {[rangeKey]: [r1, r0], [autorangeKey]: false} : {[rangeKey]: currRange}; + } + // scale switch: falls through, both bounds are taken from the current range below + } + + // fill missing bounds from the current range, in data units + if (currRange && hasOpts) { + const [d0, d1] = isCurrLog ? currRange.map((e) => 10 ** e) : currRange; + if (Number.isNaN(min)) min = Math.min(d0, d1); + if (Number.isNaN(max)) max = Math.max(d0, d1); + } + + if (Number.isNaN(min) || Number.isNaN(max)) return autoscale; // a bound with nothing to fill it from + if (isLog && (min <= 0 || max <= 0)) return autoscale; // can't show 0 or below on a log scale + return {[rangeKey]: getRange(min, max, isLog, flip), [autorangeKey]: false}; +} diff --git a/src/firefly/js/charts/__tests__/AxisRange-test.js b/src/firefly/js/charts/__tests__/AxisRange-test.js new file mode 100644 index 0000000000..6aa1893cb1 --- /dev/null +++ b/src/firefly/js/charts/__tests__/AxisRange-test.js @@ -0,0 +1,212 @@ +/*eslint-env node, jest */ + +// PlotlyToolbar pulls in canvas-based UI that jsdom can't load; BasicOptions only needs ActiveTraceSelect from it +jest.mock('firefly/charts/ui/PlotlyToolbar.jsx', () => ({ActiveTraceSelect: () => null})); +jest.mock('firefly/charts/ChartsCntlr.js', () => ({ + ...jest.requireActual('firefly/charts/ChartsCntlr.js'), + getChartData: jest.fn(), +})); + +import {getChartData} from 'firefly/charts/ChartsCntlr.js'; +import {evalChangesFromFields} from 'firefly/charts/ui/options/BasicOptions.jsx'; +import {evalAxisRangeChanges} from 'firefly/charts/AxisRangeUtil.js'; + +/** + * Pick the axis range related keys present in the changes, distinguishing "not changed" (key absent) from "set to undefined" + * @param {object} changes + * @param {string} a - axis: 'x' or 'y' + * @returns {object} + */ +function axisKeys(changes, a) { + const out = {}; + ['range', 'autorange', 'type'].forEach((p) => { + const k = `layout.${a}axis.${p}`; + if (k in changes) out[p] = changes[k]; + }); + return out; +} + +/** + * Run evalChangesFromFields against a chart with the given stored layout and pick out its axis range keys + * @param {object} layout - chart layout in the store (as synced from plotly) + * @param {object} fields - options dialog fields + * @returns {{x: object, y: object, changes: object}} per-axis range keys, plus all changes + */ +function evalChangesFromFieldsAxes(layout, fields) { + getChartData.mockReturnValue({layout, data: []}); + const changes = evalChangesFromFields('test-chart', undefined, fields); + return {x: axisKeys(changes, 'x'), y: axisKeys(changes, 'y'), changes}; +} + +const AUTO = {range: [1, 9], autorange: true}; // initial chart: plotly-computed range synced into the store +const FIXED = {range: [1, 9], autorange: false}; // e.g. after a previous Apply with bounds +const REV = {range: [9, 1], autorange: false}; // reversed +const ZOOMED = {range: [3, 4], autorange: false}; // after a zoom +const LOG = {type: 'log', range: [0, 1], autorange: true}; // log axis showing 1..10 (range is in log10 units) + +const bounds = (min, max) => ({ + ...(min !== undefined && {'fireflyLayout.yaxis.min': `${min}`}), + ...(max !== undefined && {'fireflyLayout.yaxis.max': `${max}`}), +}); + +describe('evalChangesFromFields: axis range changes', () => { + + describe('reverse without bounds', () => { + test('unchanged direction keeps the range and leaves autorange alone', () => { + expect(evalChangesFromFieldsAxes({yaxis: AUTO}, {__yoptions: ''}).y).toEqual({range: [1, 9]}); + expect(evalChangesFromFieldsAxes({yaxis: REV}, {__yoptions: 'flip'}).y).toEqual({range: [9, 1]}); + }); + test('reverse swaps the kept range and fixes it', () => { + expect(evalChangesFromFieldsAxes({yaxis: AUTO}, {__yoptions: 'flip'}).y) + .toEqual({range: [9, 1], autorange: false}); + }); + test('un-reverse swaps the kept range back', () => { + expect(evalChangesFromFieldsAxes({yaxis: REV}, {__yoptions: ''}).y) + .toEqual({range: [1, 9], autorange: false}); + }); + test('no stored range uses autorange', () => { + expect(evalChangesFromFieldsAxes({}, {__yoptions: ''}).y).toEqual({autorange: true, range: undefined}); + expect(evalChangesFromFieldsAxes({}, {__yoptions: 'flip'}).y).toEqual({autorange: 'reversed', range: undefined}); + }); + test('zoomed range is kept when applying unrelated options', () => { + expect(evalChangesFromFieldsAxes({yaxis: ZOOMED}, {__yoptions: ''}).y).toEqual({range: [3, 4]}); + }); + test('clearing bounds keeps the range they set (indistinguishable from a zoom)', () => { + const fromBounds = {range: [2, 8], autorange: false}; + expect(evalChangesFromFieldsAxes({yaxis: fromBounds}, {__yoptions: ''}).y).toEqual({range: [2, 8]}); + }); + }); + + describe('bounds', () => { + test('both bounds, other axis untouched', () => { + const {x, y} = evalChangesFromFieldsAxes({xaxis: FIXED, yaxis: FIXED}, {__yoptions: '', ...bounds(2, 8)}); + expect(y).toEqual({range: [2, 8], autorange: false}); + expect(x).toEqual({}); + }); + test('reverse with bounds (FIREFLY-2103)', () => { + expect(evalChangesFromFieldsAxes({yaxis: FIXED}, {__yoptions: 'flip', ...bounds(2, 8)}).y) + .toEqual({range: [8, 2], autorange: false}); + }); + test('un-reverse with bounds (FIREFLY-2103)', () => { + expect(evalChangesFromFieldsAxes({yaxis: REV}, {__yoptions: '', ...bounds(2, 8)}).y) + .toEqual({range: [2, 8], autorange: false}); + }); + test('missing bound is filled from the current range', () => { + expect(evalChangesFromFieldsAxes({yaxis: FIXED}, {__yoptions: '', ...bounds(2)}).y) + .toEqual({range: [2, 9], autorange: false}); + expect(evalChangesFromFieldsAxes({yaxis: FIXED}, {__yoptions: 'flip', ...bounds(undefined, 8)}).y) + .toEqual({range: [8, 1], autorange: false}); + }); + test('bounds are kept in fireflyLayout (used by heatmap binning and the dialog)', () => { + const {changes} = evalChangesFromFieldsAxes({yaxis: FIXED}, {__yoptions: '', ...bounds(2, 8)}); + expect(changes['fireflyLayout.yaxis.min']).toBe('2'); + expect(changes['fireflyLayout.yaxis.max']).toBe('8'); + }); + }); + + describe('axis column change (reset)', () => { + test('no bounds: range is dropped for autorange in the requested direction', () => { + expect(evalChangesFromFieldsAxes({yaxis: REV}, {__yoptions: '', __yreset: 'true'}).y) + .toEqual({autorange: true, range: undefined}); + expect(evalChangesFromFieldsAxes({yaxis: FIXED}, {__yoptions: 'flip', __yreset: 'true'}).y) + .toEqual({autorange: 'reversed', range: undefined}); + }); + test('bounds: direction comes from the reverse option, not the old range', () => { + expect(evalChangesFromFieldsAxes({yaxis: REV}, {__yoptions: '', __yreset: 'true', ...bounds(2, 8)}).y) + .toEqual({range: [2, 8], autorange: false}); + expect(evalChangesFromFieldsAxes({yaxis: FIXED}, {__yoptions: 'flip', __yreset: 'true', ...bounds(2, 8)}).y) + .toEqual({range: [8, 2], autorange: false}); + }); + test('a single bound with nothing to fill the other from, autoscales in the requested direction', () => { + expect(evalChangesFromFieldsAxes({yaxis: FIXED}, {__yoptions: '', __yreset: 'true', ...bounds(2, undefined)}).y) + .toEqual({autorange: true, range: undefined}); + expect(evalChangesFromFieldsAxes({yaxis: FIXED}, {__yoptions: 'flip', __yreset: 'true', ...bounds(undefined, 8)}).y) + .toEqual({autorange: 'reversed', range: undefined}); + }); + test('both axes reset together (histogram column change)', () => { + const {x, y} = evalChangesFromFieldsAxes({xaxis: FIXED, yaxis: REV}, + {__xoptions: '', __yoptions: '', __xreset: 'true', __yreset: 'true'}); + expect(x).toEqual({autorange: true, range: undefined}); + expect(y).toEqual({autorange: true, range: undefined}); + }); + }); + + describe('log scale', () => { + test('linear to log converts the current range to log10 units', () => { + const c = evalChangesFromFieldsAxes({yaxis: AUTO}, {__yoptions: 'log'}).y; + expect(c.type).toBe('log'); + expect(c.autorange).toBe(false); + expect(c.range[0]).toBeCloseTo(0); + expect(c.range[1]).toBeCloseTo(Math.log10(9)); + }); + test('log to linear converts the current range back to data units', () => { + const c = evalChangesFromFieldsAxes({yaxis: LOG}, {__yoptions: ''}).y; + expect(c.type).toBe('linear'); + expect(c.autorange).toBe(false); + expect(c.range[0]).toBeCloseTo(1); + expect(c.range[1]).toBeCloseTo(10); + }); + test('scale switch keeps a reversed axis reversed', () => { + const c = evalChangesFromFieldsAxes({yaxis: REV}, {__yoptions: 'flip,log'}).y; + expect(c.autorange).toBe(false); + expect(c.range[0]).toBeCloseTo(Math.log10(9)); + expect(c.range[1]).toBeCloseTo(0); + }); + test('log, reverse and bounds together', () => { + const c = evalChangesFromFieldsAxes({yaxis: FIXED}, {__yoptions: 'flip,log', ...bounds(2, 8)}).y; + expect(c.range[0]).toBeCloseTo(Math.log10(8)); + expect(c.range[1]).toBeCloseTo(Math.log10(2)); + }); + test('missing bound is filled from a log range in data units', () => { + const c = evalChangesFromFieldsAxes({yaxis: LOG}, {__yoptions: 'log', ...bounds(2)}).y; + expect(c.range[0]).toBeCloseTo(Math.log10(2)); + expect(c.range[1]).toBeCloseTo(1); + }); + test('staying on log keeps the range and leaves autorange alone', () => { + // used to rebuild the range and set autorange false on every Apply, freezing an autoscaled log axis + expect(evalChangesFromFieldsAxes({yaxis: LOG}, {__yoptions: 'log'}).y).toEqual({type: 'log', range: [0, 1]}); + }); + test('linear to log with a range reaching 0 or below falls back to autorange', () => { + // used to produce a NaN range + expect(evalChangesFromFieldsAxes({yaxis: {range: [-5, 9], autorange: true}}, {__yoptions: 'log'}).y) + .toEqual({type: 'log', autorange: true, range: undefined}); + expect(evalChangesFromFieldsAxes({yaxis: {range: [-5, 9], autorange: true}}, {__yoptions: 'log,flip'}).y) + .toEqual({type: 'log', autorange: 'reversed', range: undefined}); + }); + }); + + describe('fields without the axis options (e.g. spectrum unit conversion)', () => { + test('no axis range fields leave the range alone', () => { + expect(evalChangesFromFieldsAxes({yaxis: REV}, {'layout.yaxis.title.text': 'flux'}).y).toEqual({}); + }); + test('bounds without the reverse option keep the current direction', () => { + expect(evalChangesFromFieldsAxes({yaxis: REV}, bounds(2, 8)).y).toEqual({range: [8, 2], autorange: false}); + expect(evalChangesFromFieldsAxes({yaxis: FIXED}, bounds(2, 8)).y).toEqual({range: [2, 8], autorange: false}); + }); + }); + + test('x axis behaves the same', () => { + const c = evalChangesFromFieldsAxes({xaxis: FIXED}, + {__xoptions: 'flip', 'fireflyLayout.xaxis.min': '2', 'fireflyLayout.xaxis.max': '8'}).x; + expect(c).toEqual({range: [8, 2], autorange: false}); + }); +}); + +describe('evalAxisRangeChanges', () => { + test('reset without axis options or bounds leaves the axis alone', () => { + expect(evalAxisRangeChanges({a: 'y', fields: {__yreset: 'true'}, axisLayout: REV})).toEqual({}); + }); + test('axis options key present with no value means nothing checked', () => { + expect(evalAxisRangeChanges({a: 'y', fields: {__yoptions: undefined}, axisLayout: REV})) + .toEqual({'layout.yaxis.range': [1, 9], 'layout.yaxis.autorange': false}); + }); + test('bounds without axis options use the current scale', () => { + const c = evalAxisRangeChanges({a: 'y', fields: bounds(2, 8), axisLayout: LOG}); + expect(c['layout.yaxis.range'][0]).toBeCloseTo(Math.log10(2)); + expect(c['layout.yaxis.range'][1]).toBeCloseTo(Math.log10(8)); + }); + test('no axis layout at all', () => { + expect(evalAxisRangeChanges({a: 'y', fields: {__yoptions: 'flip'}})) + .toEqual({'layout.yaxis.autorange': 'reversed', 'layout.yaxis.range': undefined}); + }); +}); diff --git a/src/firefly/js/charts/ui/options/BasicOptions.jsx b/src/firefly/js/charts/ui/options/BasicOptions.jsx index ba8af7d738..2acd45fd8e 100644 --- a/src/firefly/js/charts/ui/options/BasicOptions.jsx +++ b/src/firefly/js/charts/ui/options/BasicOptions.jsx @@ -1,6 +1,6 @@ import React, {useCallback, useEffect} from 'react'; import PropTypes from 'prop-types'; -import {get, isArray, isEmpty, isString, isUndefined, memoize, reverse, set} from 'lodash'; +import {get, isArray, isEmpty, isString, isUndefined, set} from 'lodash'; import {DEFAULT_PLOT2D_VIEWER_ID} from '../../../visualize/VisConst'; import {dispatchChartAdd, dispatchChartUpdate, getChartData} from '../../ChartsCntlr.js'; @@ -24,6 +24,7 @@ import {hideColSelectPopup} from '../ColSelectView.jsx'; import {addColorbarChanges} from '../../dataTypes/FireflyHeatmap.js'; import {colorsOnTypes, getChartProps, toRGBA, TRACE_COLORS, uniqueChartId, isSpectrum} from '../../ChartUtil.js'; import {colorscaleNameToVal} from '../../Colorscale.js'; +import {evalAxisRangeChanges, isAxisReversed} from '../../AxisRangeUtil.js'; import MAGNIFYING_GLASS from 'html/images/icons-2014/magnifyingGlass.png'; import {ToolbarButton} from '../../../ui/ToolbarButton.jsx'; @@ -55,10 +56,7 @@ function getOptions(a, layout) { if ( (isUndefined(showgrid) && get(layout, `${a}axis.gridwidth`)) || showgrid) { opts.push('grid'); } - const range = get(layout, `${a}axis.range`) || []; - const autorange = get(layout, `${a}axis.autorange`); - const reversed = (autorange === 'reversed') || (range[1] < range[0]); - if (reversed) { + if (isAxisReversed(get(layout, `${a}axis`))) { opts.push('flip'); } if (get(layout, `${a}axis.side`) === (a==='x'?'top':'right')) { @@ -320,30 +318,6 @@ export function evalChangesFromFields(chartId, tbl_id, fields) { ['x','y'].forEach((a) => { if (k === `__${a}options`) { const opts = v || ''; - - // helper hidden fields __xreset and __yreset keep track of whether x and y have changed - // if this field is set, we'd like to clear range - const range = !fields[`__${a}reset`] && get(layout, `${a}axis.range`); - - if (opts.includes('flip')) { - if (range) { - // reverse mutates the original array - changes[`layout.${a}axis.range`] = (range[0] { - let minUser = parseFloat(get(changes, `fireflyLayout.${a}axis.min`)); - let maxUser = parseFloat(get(changes, `fireflyLayout.${a}axis.max`)); - - if (!Number.isNaN(minUser) || !Number.isNaN(maxUser) || changes[`layout.${a}axis.type`]) { - // range values of a log axis are logs - convert them back - const range = changes[`layout.${a}axis.range`] && - (get(layout, `${a}axis.range`, []).map(get(layout, `${a}axis.type`) === 'log' ? (e)=>Math.pow(10, e) : (e)=>e)); - if (Number.isNaN(minUser) && range) { - minUser = Math.min(range[0], range[1]); - } - if (Number.isNaN(maxUser) && range) { - maxUser = Math.max(range[0], range[1]); - } - - if (!Number.isNaN(minUser) || !Number.isNaN(maxUser)) { - const autorange = changes[`layout.${a}axis.autorange`]; - const reversed = (autorange === 'reversed') || (!autorange && range[1] < range[0]); - changes[`layout.${a}axis.range`] = getRange(minUser, maxUser, changes[`layout.${a}axis.type`] === 'log', reversed); - changes[`layout.${a}axis.autorange`] = false; - } - } - }); -} - - -/** - * Get range for a plotly axis - * Plotly requires range to be reversed if the axis is reversed, - * and limits to be log if axis scale is log - * @param min - minimum value - * @param max - maximum value - * @param isLog - true, if an axis uses log scale - * @param isReversed - true, if the axis should be reversed - * @returns {Array} an array for axis range property in plotly layout - */ -function getRange(min, max, isLog, isReversed) { - const [r1, r2] = isReversed ? [max, min] : [min, max]; - return isLog ? [Math.log10(r1), Math.log10(r2)] : [r1, r2]; + return { + ...changes, + ...evalAxisRangeChanges({a: 'x', fields, axisLayout: layout.xaxis}), + ...evalAxisRangeChanges({a: 'y', fields, axisLayout: layout.yaxis}), + }; } /**