Skip to content

DM-6954 XY plot problems found #137

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

Merged
merged 1 commit into from
Aug 8, 2016
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
Binary file added src/firefly/html/images/down-pointer.gif
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
91 changes: 78 additions & 13 deletions src/firefly/js/charts/XYPlotCntlr.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
*/
import {flux} from '../Firefly.js';

import {updateSet, updateMerge} from '../util/WebUtil.js';
import {get, has, omit, omitBy, isUndefined, isString} from 'lodash';
import {updateSet, updateMerge, updateDelete} from '../util/WebUtil.js';
import {get, has, omit, omitBy, isEmpty, isUndefined, isString} from 'lodash';

import {doFetchTable, getColumn, getTblById, isFullyLoaded, cloneRequest} from '../tables/TableUtil.js';
import * as TablesCntlr from '../tables/TablesCntlr.js';
Expand Down Expand Up @@ -149,7 +149,8 @@ export function loadPlotData (rawAction) {

if (!serverCallNeeded) {
const tableModel = getTblById(tblId);
xyPlotParams = getUpdatedParams(xyPlotParams, tableModel);
const dataBoundaries = getDataBoundaries(chartModel.xyPlotData);
xyPlotParams = getUpdatedParams(xyPlotParams, tableModel, dataBoundaries);
}

dispatch({ type : LOAD_PLOT_DATA, payload : {chartId, tblId, xyPlotParams, tblSource, serverCallNeeded}});
Expand Down Expand Up @@ -188,6 +189,20 @@ function updatePlotData(data) {

export function reduceXYPlot(state={}, action={}) {
switch (action.type) {
case (TablesCntlr.TABLE_LOADED) :
{
const {tbl_id, invokedBy} = action.payload;
let updatedState = state;
if (invokedBy !== TablesCntlr.TABLE_SORT) {
Object.keys(state).forEach((cid) => {
if (state[cid].tblId === tbl_id && get(state, [cid, 'xyPlotParams', 'boundaries'])) {
// do we need hard boundaries, which do not go away on new table data?
updatedState = updateDelete(updatedState, [cid, 'xyPlotParams'], 'boundaries');
}
});
}
return updatedState;
}
case (TablesCntlr.TABLE_REMOVE) :
{
const tbl_id = action.payload.tbl_id;
Expand Down Expand Up @@ -287,8 +302,46 @@ function getServerCallParameters(xyPlotParams) {
return [xyPlotParams.x.columnOrExpr, xyPlotParams.y.columnOrExpr, maxBins, xyRatio, xMin, xMax, yMin, yMax];
}

function getDataBoundaries(xyPlotData) {
if (!isEmpty(xyPlotData)) {
return omitBy({
xMin: xyPlotData.xMin,
xMax: xyPlotData.xMax,
yMin: xyPlotData.yMin,
yMax: xyPlotData.yMax
}, isUndefined);
}
}


/**
* Pad and round data boundaries
* @param {Object} boundaries - object with xMin, xMax, yMin, yMax props
* @param {Number} factor - part of the range to add on both sides
*/
function getPaddedBoundaries(boundaries, factor=100) {
if (!isEmpty(boundaries)) {
let {xMin, xMax, yMin, yMax} = boundaries;
const xRange = xMax - xMin;

if (xRange > 0) {
const xPad = xRange/factor;
xMin = xMin - xPad;
xMax = xMax + xPad;
}
const yRange = yMax - yMin;
if (yRange > 0) {
const yPad = yRange/factor;
yMin = yMin - yPad;
yMax = yMax + yPad;
}
if (xRange > 0 || yRange > 0) {
return {xMin, xMax, yMin, yMax};
}
}
return boundaries;
}

/**
* fetches xy plot data
* set isColStatsReady to true once done.
Expand Down Expand Up @@ -352,7 +405,7 @@ function fetchPlotData(dispatch, tblId, xyPlotParams, chartId) {
xyPlotParams,
xyPlotData,
chartId,
newParams: getUpdatedParams(xyPlotParams, tableModel)
newParams: getUpdatedParams(xyPlotParams, tableModel, getDataBoundaries(xyPlotData))
}));
}
).catch(
Expand All @@ -368,32 +421,44 @@ function fetchPlotData(dispatch, tblId, xyPlotParams, chartId) {
* derive them from existing parameters or tableModel.
* No selection should be present in updated parameters
*/
function getUpdatedParams(xyPlotParams, tableModel) {
function getUpdatedParams(xyPlotParams, tableModel, dataBoundaries) {
let newParams = xyPlotParams;

if (!get(xyPlotParams, 'x.label')) {
newParams = updateSet(newParams, 'x.label', get(xyPlotParams, 'x.columnOrExpr'));
}
if (!get(xyPlotParams, 'x.unit')) {
const xColumn = getColumn(tableModel, get(xyPlotParams, 'x.columnOrExpr'));
const xUnit = get(xColumn, 'units');
if (xUnit) {
newParams = updateSet(newParams, 'x.unit', xUnit);
}
const xUnit = get(xColumn, 'units', '');
newParams = updateSet(newParams, 'x.unit', xUnit);
}
if (!get(xyPlotParams, 'y.label')) {
newParams = updateSet(newParams, 'y.label', get(xyPlotParams, 'y.columnOrExpr'));
}
if (!get(xyPlotParams, 'y.unit')) {
const yColumn = getColumn(tableModel, get(xyPlotParams, 'y.columnOrExpr'));
const yUnit = get(yColumn, 'units');
if (yUnit) {
newParams = updateSet(newParams, 'y.unit', yUnit);
}
const yUnit = get(yColumn, 'units', '');
newParams = updateSet(newParams, 'y.unit', yUnit);
}
if (get(xyPlotParams, 'selection')) {
newParams = updateSet(newParams, 'selection', undefined);
}

// set plot boundaries,
// if user set boundaries are undefined, use data boundaries
const userSetBoundaries = get(xyPlotParams, 'userSetBoundaries', {});
const boundaries = Object.assign({}, userSetBoundaries);
if (Object.keys(boundaries).length < 4 && !isEmpty(dataBoundaries)) {
const paddedDataBoundaries = getPaddedBoundaries(dataBoundaries);
const [xMin, xMax, yMin, yMax] = ['xMin', 'xMax', 'yMin', 'yMax'].map( (v) => {
return (Number.isFinite(boundaries[v]) ? boundaries[v] : paddedDataBoundaries[v]);
});
const newBoundaries = omitBy({xMin, xMax, yMin, yMax}, isUndefined);
if (!isEmpty(newBoundaries)) {
newParams = updateSet(newParams, 'boundaries', newBoundaries);
}
}

return newParams;
}

Expand Down
13 changes: 10 additions & 3 deletions src/firefly/js/charts/ui/ColSelectView.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {dispatchTableRemove} from '../../tables/TablesCntlr';

import {BasicTable} from '../../tables/ui/BasicTable.jsx';
import {getTblById} from '../../tables/TableUtil.js';
import {dispatchShowDialog} from '../../core/ComponentCntlr.js';
import {dispatchShowDialog, dispatchHideDialog, isDialogVisible} from '../../core/ComponentCntlr.js';
import CompleteButton from '../../ui/CompleteButton.jsx';
//import HelpIcon from '../../ui/HelpIcon.jsx';
const popupId = 'XYColSelect';
Expand Down Expand Up @@ -35,8 +35,9 @@ const closeButtonStyle = {'textAlign': 'center', display: 'inline-block', height

export function showColSelectPopup(colValStats,onColSelected,popupTitle,buttonText,currentVal) {

if (getTblById(TBL_ID)) {
        dispatchTableRemove(TBL_ID);
if (getTblById(TBL_ID)) {
hideColSelectPopup();
       dispatchTableRemove(TBL_ID);
    }

const colNames = colValStats.map((colVal) => {return colVal.name;});
Expand Down Expand Up @@ -72,6 +73,12 @@ export function showColSelectPopup(colValStats,onColSelected,popupTitle,buttonTe
dispatchShowDialog(popupId);
}

export function hideColSelectPopup() {
if (isDialogVisible(popupId)) {
dispatchHideDialog(popupId);
}
}

function popupForm(tableModel, onColSelected,buttonText,popupId) {
const tblId = tableModel.tbl_id;
return (
Expand Down
38 changes: 27 additions & 11 deletions src/firefly/js/charts/ui/XYPlot.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ export const plotParamsShape = PropTypes.shape({
stretch : PropTypes.oneOf(['fit','fill']),
selection : selectionShape,
zoom : selectionShape,
boundaries : selectionShape,
userSetBoundaries : selectionShape,
nbins : PropTypes.shape({x : PropTypes.number, y : PropTypes.number}),
shading : PropTypes.oneOf(['lin', 'log']),
x : axisParamsShape,
Expand Down Expand Up @@ -107,7 +109,7 @@ const getWeightedDataDescr = function(defaultDescr, numericData, minWeight, maxW
};

const getXAxisOptions = function(params) {
const xTitle = params.x.label + (params.x.unit ? ', ' + params.x.unit : '');
const xTitle = params.x.label + (params.x.unit ? ` (${params.x.unit})` : '');
let xGrid = false, xReversed = false, xLog = false;
const {options:xOptions} = params.x;
if (xOptions) {
Expand All @@ -119,7 +121,7 @@ const getXAxisOptions = function(params) {
};

const getYAxisOptions = function(params) {
const yTitle = params.y.label + (params.y.unit ? ', ' + params.y.unit : '');
const yTitle = params.y.label + (params.y.unit ? ` (${params.y.unit})` : '');

let yGrid = false, yReversed = false, yLog = false;
const {options:yOptions} = params.y;
Expand Down Expand Up @@ -275,12 +277,19 @@ export class XYPlot extends React.Component {
type: newYOptions.yLog ? 'logarithmic' : 'linear'
});
}
if (!shallowequal(params.zoom, newParams.zoom)) {
if (!shallowequal(params.boundaries, newParams.boundaries)) {
const {xMin, xMax, yMin, yMax} = getZoomSelection(newParams);
const {xMin:xDataMin, xMax:xDataMax, yMin:yDataMin, yMax:yDataMax} = nextProps.data;
Object.assign(xoptions, {min: selFinite(xMin,xDataMin), max: selFinite(xMax, xDataMax)});
Object.assign(yoptions, {min: selFinite(yMin, yDataMin), max: selFinite(yMax, yDataMax)});
}
if (!shallowequal(params.zoom, newParams.zoom) ||
!shallowequal(params.boundaries, newParams.boundaries)) {
const {xMin, xMax, yMin, yMax} = getZoomSelection(newParams);
const {xMin:xDataMin, xMax:xDataMax, yMin:yDataMin, yMax:yDataMax} = get(newParams, 'boundaries', {});
Object.assign(xoptions, {min: selFinite(xMin,xDataMin), max: selFinite(xMax, xDataMax)});
Object.assign(yoptions, {min: selFinite(yMin, yDataMin), max: selFinite(yMax, yDataMax)});
}
const xUpdate = Reflect.ownKeys(xoptions).length > 0;
const yUpdate = Reflect.ownKeys(yoptions).length > 0;
if (xUpdate || yUpdate) {
Expand Down Expand Up @@ -439,7 +448,7 @@ export class XYPlot extends React.Component {
}, {selected: [], all: []});


marker = {symbol: 'circle'};
marker = {symbol: 'circle', radius: 3};
allSeries = [
{
id: DATAPOINTS,
Expand Down Expand Up @@ -510,7 +519,7 @@ export class XYPlot extends React.Component {
id: HIGHLIGHTED,
name: HIGHLIGHTED,
color: highlightedColor,
marker: {symbol: 'circle', lineColor: '#404040', lineWidth: 1, radius: 5},
marker: {symbol: 'circle', lineColor: '#404040', lineWidth: 1, radius: 4},
data: highlightedData,
showInLegend: false
}, true, false);
Expand All @@ -527,7 +536,8 @@ export class XYPlot extends React.Component {
const {xTitle, xGrid, xReversed, xLog} = getXAxisOptions(params);
const {yTitle, yGrid, yReversed, yLog} = getYAxisOptions(params);
const {xMin, xMax, yMin, yMax} = getZoomSelection(params);
const {xMin:xDataMin, xMax:xDataMax, yMin:yDataMin, yMax:yDataMax, decimateKey} = data;
const {decimateKey} = data;
const {xMin:xDataMin, xMax:xDataMax, yMin:yDataMin, yMax:yDataMax} = get(params, 'boundaries', {});

const makeSeries = this.makeSeries;

Expand Down Expand Up @@ -635,16 +645,22 @@ export class XYPlot extends React.Component {
type: yLog ? 'logarithmic' : 'linear'
},
series: [{
// this series is to make sure the axis are created
// without actual series xAxis creation is deferred
// This series is to make sure the axis are created.
// Without actual series, xAxis creation is deferred
// and there is no way to get value to pixel conversion
// for sizing the symbol
id: 'minmax',
name: 'minmax',
color: '#f0f0f0',
marker: {radius: 2},
color: 'rgba(240, 240, 240, 0.1)',
marker: {radius: 1},
data: [[selFinite(xMin, xDataMin), selFinite(yMin,yDataMin)], [selFinite(xMax, xDataMax), selFinite(yMax,yDataMax)]],
showInLegend: false
showInLegend: false,
enableMouseTracking: false,
states: {
hover: {
enabled: false
}
}
}],
credits: {
enabled: false // removes a reference to Highcharts.com from the chart
Expand Down
Loading