Skip to content

Commit 487a566

Browse files
tgoldinatgoldina
authored
DM-9590 merge XY plot is unrecoverable after failure, pr #329
DM-9590 XY plot is unrecoverable after it fails; histogram behavior should match xy plot behavior
2 parents 9340db0 + 5b8c1fc commit 487a566

File tree

7 files changed

+124
-68
lines changed

7 files changed

+124
-68
lines changed

‎src/firefly/java/edu/caltech/ipac/firefly/server/query/HistogramProcessor.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -306,11 +306,14 @@ private boolean isInSelection( int idx, ArrayList<Integer> list){
306306
return false;
307307
}
308308

309-
private double[] getColumnData(DataGroup dg) {
309+
private double[] getColumnData(DataGroup dg) throws DataAccessException {
310310
List<DataObject> objList = dg.values();
311311
int nRow = objList.size();
312312
DataType[] dataTypes=dg.getDataDefinitions();
313313
DataObjectUtil.DoubleValueGetter dGetter = new DataObjectUtil.DoubleValueGetter(dataTypes, columnExpression);
314+
if (!dGetter.isValid()) {
315+
throw new DataAccessException("Invalid column or expression: "+columnExpression);
316+
}
314317

315318
double[] data = new double[nRow];
316319
for (int i = 0; i < nRow; i++) {

‎src/firefly/java/edu/caltech/ipac/firefly/server/util/QueryUtil.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -623,7 +623,7 @@ public static DataAccessException createEndUserException(String msg, String deta
623623
* @param decimateInfo DecimateInfo object
624624
* @return decimated data group
625625
*/
626-
public static DataGroup doDecimation(DataGroup dg, DecimateInfo decimateInfo) {
626+
public static DataGroup doDecimation(DataGroup dg, DecimateInfo decimateInfo) throws DataAccessException {
627627

628628
double xMax = Double.NEGATIVE_INFINITY, xMin = Double.POSITIVE_INFINITY, yMax = Double.NEGATIVE_INFINITY, yMin = Double.POSITIVE_INFINITY;
629629

@@ -633,7 +633,7 @@ public static DataGroup doDecimation(DataGroup dg, DecimateInfo decimateInfo) {
633633

634634
if (!xValGetter.isValid() || !yValGetter.isValid()) {
635635
System.out.println("QueryUtil.doDecimation: invalid x or y column.");
636-
return null; // TODO: handle null return in the caller?
636+
throw new DataAccessException("Invalid column or expression");
637637
}
638638

639639
int maxPoints = decimateInfo.getMaxPoints() == 0 ? DECI_DEF_MAX_POINTS : decimateInfo.getMaxPoints();

‎src/firefly/js/charts/dataTypes/HistogramCDT.js

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -169,16 +169,25 @@ function fetchColData(dispatch, chartId, chartDataElementId) {
169169
}
170170
).catch(
171171
(reason) => {
172-
const message = 'Failed to fetch histogram data';
173-
logError(`${message}: ${reason}`);
174-
dispatch(chartDataUpdate(
175-
{
176-
chartId,
177-
chartDataElementId,
178-
isDataReady: true,
179-
error: {message, reason},
180-
data: undefined
181-
}));
172+
dispatchError(dispatch, chartId, chartDataElementId, reason);
182173
}
183174
);
184175
}
176+
function dispatchError(dispatch, chartId, chartDataElementId, reason) {
177+
const message = 'Failed to fetch histogram data';
178+
logError(`${message}: ${reason}`);
179+
let reasonStr = `${reason}`.toLowerCase();
180+
if (reasonStr.match(/invalid column/)) {
181+
reasonStr = 'Non-existent column or invalid expression. Please use valid value.';
182+
} else {
183+
reasonStr = 'Please contact Help Desk. Check browser console for more information.';
184+
}
185+
dispatch(chartDataUpdate(
186+
{
187+
chartId,
188+
chartDataElementId,
189+
isDataReady: true,
190+
error: {message, reason: reasonStr},
191+
data: undefined
192+
}));
193+
}

‎src/firefly/js/charts/dataTypes/XYColsCDT.js

Lines changed: 52 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -219,20 +219,19 @@ function serverParamsChanged(oldParams, newParams, chartDataElement) {
219219
} else {
220220
// 'x', 'y', 'sortBy', 'xErr', 'xErrLow', 'xErrHigh', 'yErr', 'yErrLow', 'yErrHigh'
221221
// server call parameters are present in the data
222-
const newOpts = omitBy({
223-
sortBy: newParams.sortColOrExpr,
224-
x: newParams.x.columnOrExpr,
225-
xErr: newParams.x.error,
226-
xErrLow: newParams.x.errorLow,
227-
xErrHigh: newParams.x.errorHigh,
228-
y: newParams.y.columnOrExpr,
229-
yErr: newParams.y.error,
230-
yErrLow: newParams.y.errorLow,
231-
yErrHigh: newParams.y.errorHigh
232-
}, isUndefined);
233-
return Object.keys(newOpts).some((o) => {
234-
return newOpts[o] !== data[o];
235-
});
222+
const newOpts = getServerCallParameters(newParams, false);
223+
if (data) {
224+
// if data available, see if the new parameters are different from those used to obtain the data
225+
return Object.keys(newOpts).some((o) => {
226+
return newOpts[o] !== data[o];
227+
});
228+
} else {
229+
// if data are not available, compare with the old parameters
230+
const oldOpts = getServerCallParameters(oldParams, false);
231+
return Object.keys(newOpts).some((o) => {
232+
return newOpts[o] !== oldOpts[o];
233+
});
234+
}
236235
}
237236
}
238237

@@ -249,22 +248,39 @@ function isLargeTable(tblId) {
249248
// xyPlotParams.x.errorHigh || xyPlotParams.y.errorHigh);
250249
//}
251250

252-
function getServerCallParameters(xyPlotParams) {
253-
if (!xyPlotParams) { return []; }
251+
function getServerCallParameters(xyPlotParams, isLargeTable=true) {
252+
if (isLargeTable) {
253+
if (!xyPlotParams) {
254+
return [];
255+
}
254256

255-
if (xyPlotParams.zoom) {
256-
var {xMin, xMax, yMin, yMax} = xyPlotParams.zoom;
257-
}
257+
if (xyPlotParams.zoom) {
258+
var {xMin, xMax, yMin, yMax} = xyPlotParams.zoom;
259+
}
258260

259-
let maxBins = 10000;
260-
let xyRatio = xyPlotParams.xyRatio || 1.0;
261-
if (xyPlotParams.nbins) {
262-
const {x, y} = xyPlotParams.nbins;
263-
maxBins = x*y;
264-
xyRatio = x/y;
261+
let maxBins = 10000;
262+
let xyRatio = xyPlotParams.xyRatio || 1.0;
263+
if (xyPlotParams.nbins) {
264+
const {x, y} = xyPlotParams.nbins;
265+
maxBins = x * y;
266+
xyRatio = x / y;
267+
}
268+
// order should match the order of the parameters in serializeDecimateInfo
269+
return [xyPlotParams.x.columnOrExpr, xyPlotParams.y.columnOrExpr, maxBins, xyRatio, xMin, xMax, yMin, yMax];
270+
} else {
271+
// smaller (not decimated) table
272+
return omitBy({
273+
sortBy: xyPlotParams.sortColOrExpr,
274+
x: xyPlotParams.x.columnOrExpr,
275+
xErr: xyPlotParams.x.error,
276+
xErrLow: xyPlotParams.x.errorLow,
277+
xErrHigh: xyPlotParams.x.errorHigh,
278+
y: xyPlotParams.y.columnOrExpr,
279+
yErr: xyPlotParams.y.error,
280+
yErrLow: xyPlotParams.y.errorLow,
281+
yErrHigh: xyPlotParams.y.errorHigh
282+
}, isUndefined);
265283
}
266-
// order should match the order of the parameters in serializeDecimateInfo
267-
return [xyPlotParams.x.columnOrExpr, xyPlotParams.y.columnOrExpr, maxBins, xyRatio, xMin, xMax, yMin, yMax];
268284
}
269285

270286
export function getDataBoundaries(xyPlotData) {
@@ -574,12 +590,20 @@ function fetchXYWithErrorsOrSort(dispatch, chartId, chartDataElementId) {
574590
function dispatchError(dispatch, chartId, chartDataElementId, reason) {
575591
const message = 'Failed to fetch XY plot data';
576592
logError(`${message}: ${reason}`);
593+
let reasonStr = `${reason}`.toLowerCase();
594+
if (reasonStr.match(/not supported/)) {
595+
reasonStr = 'Unsupported feature requested. Please choose valid options.';
596+
} else if (reasonStr.match(/invalid column/)) {
597+
reasonStr = 'Non-existent column or invalid expression. Please choose valid X and Y.';
598+
} else {
599+
reasonStr = 'Please contact Help Desk. Check browser console for more information.';
600+
}
577601
dispatch(chartDataUpdate(
578602
{
579603
chartId,
580604
chartDataElementId,
581605
isDataReady: true,
582-
error: {message, reason},
606+
error: {message, reason: reasonStr},
583607
data: undefined
584608
}));
585609
}

‎src/firefly/js/charts/ui/ColumnOrExpression.jsx

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import React, {PropTypes} from 'react';
55

66
import {get} from 'lodash';
7+
import {Expression} from '../../util/expr/Expression.js';
78
import {dispatchValueChange} from '../../fieldGroup/FieldGroupCntlr.js';
89
import {TextButton} from '../../ui/TextButton.jsx';
910
import {SuggestBoxInputField} from '../../ui/SuggestBoxInputField.jsx';
@@ -29,6 +30,24 @@ function parseSuggestboxContent(text) {
2930
return {token, priorContent};
3031
}
3132

33+
export function getColValidator(colValStats, required=true) {
34+
const colNames = colValStats.map((colVal) => {return colVal.name;});
35+
return (val) => {
36+
let retval = {valid: true, message: ''};
37+
if (!val) {
38+
if (required) {
39+
return {valid: false, message: 'Can not be empty. Please provide value or expression'};
40+
}
41+
} else if (colNames.indexOf(val) < 0) {
42+
const expr = new Expression(val, colNames);
43+
if (!expr.isValid()) {
44+
retval = {valid: false, message: `${expr.getError().error}. Unable to parse ${val}.`};
45+
}
46+
}
47+
return retval;
48+
};
49+
}
50+
3251
export function ColumnOrExpression({colValStats,params,groupKey,fldPath,label,labelWidth=30,tooltip,nullAllowed}) {
3352

3453
// the suggestions are indexes in the colValStats array - it makes it easier to render then with labels
@@ -55,12 +74,18 @@ export function ColumnOrExpression({colValStats,params,groupKey,fldPath,label,la
5574
val = colName;
5675
dispatchValueChange({fieldKey: fldPath, groupKey, value: colName, valid: true});
5776
};
77+
const colValidator = getColValidator(colValStats,!nullAllowed);
78+
const value = get(params, fldPath);
79+
const {valid=true, message=''} = value ? colValidator(value) : {};
5880
return (
5981
<div style={{whiteSpace: 'nowrap'}}>
6082
<SuggestBoxInputField
6183
inline={true}
6284
initialState= {{
63-
value: get(params, fldPath),
85+
value,
86+
valid,
87+
message,
88+
validator: colValidator,
6489
tooltip: `Column or expression for ${tooltip}`,
6590
label: `${label}:`,
6691
nullAllowed

‎src/firefly/js/charts/ui/HistogramOptions.jsx

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,20 @@ import {DATATYPE_HISTOGRAM} from '../dataTypes/HistogramCDT.js';
66
import CompleteButton from '../../ui/CompleteButton.jsx';
77
import {FieldGroup} from '../../ui/FieldGroup.jsx';
88
import FieldGroupUtils from '../../fieldGroup/FieldGroupUtils.js';
9-
import {dispatchMultiValueChange} from '../../fieldGroup/FieldGroupCntlr.js';
9+
import {dispatchValueChange, dispatchMultiValueChange} from '../../fieldGroup/FieldGroupCntlr.js';
1010
import {InputGroup} from '../../ui/InputGroup.jsx';
1111
import Validate from '../../util/Validate.js';
1212
import {ValidationField} from '../../ui/ValidationField.jsx';
1313
import {CheckboxGroupInputField} from '../../ui/CheckboxGroupInputField.jsx';
1414
import {RadioGroupInputField} from '../../ui/RadioGroupInputField.jsx';
1515
import {FieldGroupCollapsible} from '../../ui/panel/CollapsiblePanel.jsx';
16-
import {ColumnOrExpression} from './ColumnOrExpression.jsx';
16+
import {ColumnOrExpression, getColValidator} from './ColumnOrExpression.jsx';
1717
import {getAppOptions} from '../../core/AppDataCntlr.js';
1818

1919

2020
export const histogramParamsShape = PropTypes.shape({
2121
algorithm : PropTypes.oneOf(['fixedSizeBins','bayesianBlocks']),
22-
numBins : PropTypes.string,
22+
numBins : PropTypes.oneOfType([React.PropTypes.string,React.PropTypes.number]),
2323
falsePositiveRate : PropTypes.string,
2424
minCutoff : PropTypes.number,
2525
maxCutoff : PropTypes.number
@@ -85,6 +85,18 @@ export class HistogramOptions extends React.Component {
8585
this.setState({fields});
8686
}
8787
});
88+
// make sure column validator matches current columns
89+
const {colValStats, groupKey, histogramParams} = this.props;
90+
if (colValStats) {
91+
const colValidator = getColValidator(colValStats);
92+
var payload = {groupKey, fieldKey: 'columnOrExpr', validator: colValidator};
93+
const value = get(histogramParams, 'columnOrExpr');
94+
if (value) {
95+
var {valid, message} = colValidator(value);
96+
payload = Object.assign(payload, {value, valid, message});
97+
}
98+
dispatchValueChange(payload);
99+
}
88100
this.iAmMounted= true;
89101
}
90102

@@ -133,11 +145,13 @@ export class HistogramOptions extends React.Component {
133145
const { colValStats, groupKey, histogramParams, defaultParams, onOptionsSelected} = this.props;
134146
const {fixedAlgorithm=false} = this.state;
135147
const xProps = {colValStats,params:histogramParams,groupKey,fldPath:'columnOrExpr',label:'Column or expression', labelWidth:120, tooltip:'X Axis',nullAllowed:false};
136-
148+
137149
const algorithm = get(histogramParams, 'algorithm', 'fixedSizeBins');
138150
const m_algorithmOptions = fixedAlgorithm ? algorithmOptions.filter((el) => el.value === algorithm) : algorithmOptions;
151+
// set minimum height to fit full height suggest box,
152+
// to avoid width change due to scroll bar appearing when full height suggest box is rendered
139153
return (
140-
<div style={{padding:'0 5px'}}>
154+
<div style={{padding:'0 5px', minHeight: 250}}>
141155
<FieldGroup groupKey={groupKey} validatorFunc={null} keepState={true}>
142156
{onOptionsSelected &&
143157
<div style={{display: 'flex', flexDirection: 'row', padding: '5px 0 15px'}}>

‎src/firefly/js/charts/ui/XYPlotOptions.jsx

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,13 @@ import {FieldGroup} from '../../ui/FieldGroup.jsx';
1111
import FieldGroupUtils from '../../fieldGroup/FieldGroupUtils.js';
1212
import {dispatchMultiValueChange, VALUE_CHANGE, MULTI_VALUE_CHANGE} from '../../fieldGroup/FieldGroupCntlr.js';
1313
import Validate from '../../util/Validate.js';
14-
import {Expression} from '../../util/expr/Expression.js';
1514
import {ValidationField} from '../../ui/ValidationField.jsx';
1615
import {CheckboxGroupInputField} from '../../ui/CheckboxGroupInputField.jsx';
1716
import {RadioGroupInputField} from '../../ui/RadioGroupInputField.jsx';
1817
import {FieldGroupCollapsible} from '../../ui/panel/CollapsiblePanel.jsx';
1918
import {plotParamsShape} from './XYPlot.jsx';
2019
import {hideColSelectPopup} from './ColSelectView.jsx';
21-
import {ColumnOrExpression} from './ColumnOrExpression.jsx';
20+
import {ColumnOrExpression, getColValidator} from './ColumnOrExpression.jsx';
2221
import {updateSet} from '../../util/WebUtil.js';
2322

2423
const DECI_ENABLE_SIZE = 5000;
@@ -113,7 +112,7 @@ export function resultsSuccess(callback, flds, optionParameters) {
113112
x : { columnOrExpr : xName, error: xErr, label : xLabel, unit : xUnit, options : xOptions},
114113
y : { columnOrExpr : yName, error: yErr, label : yLabel, unit : yUnit, options : yOptions},
115114
tblId,
116-
zoom,
115+
zoom
117116
}, isUndefined);
118117

119118
if (xErr || yErr) {
@@ -155,24 +154,6 @@ export function setOptions(groupKey, xyPlotParams) {
155154
dispatchMultiValueChange(groupKey, flds);
156155
}
157156

158-
export function getColValidator(colValStats, required=true) {
159-
const colNames = colValStats.map((colVal) => {return colVal.name;});
160-
return (val) => {
161-
let retval = {valid: true, message: ''};
162-
if (!val) {
163-
if (required) {
164-
return {valid: false, message: 'Can not be empty. Please provide value or expression'};
165-
}
166-
} else if (colNames.indexOf(val) < 0) {
167-
const expr = new Expression(val, colNames);
168-
if (!expr.isValid()) {
169-
retval = {valid: false, message: `${expr.getError().error}. Unable to parse ${val}.`};
170-
}
171-
}
172-
return retval;
173-
};
174-
}
175-
176157
/**
177158
* Reducer from field group component,
178159
* clears label, unit, and userSetBoundaries whenever x or y field changes,

0 commit comments

Comments
 (0)