From 110b32a857743e1dcd6a27a3b7dd4a971d3a36b0 Mon Sep 17 00:00:00 2001 From: Matthew McDermott Date: Tue, 12 Jul 2016 12:33:23 -0700 Subject: [PATCH 01/12] Added tooltip to display errors. --- .../distributions/editor/TextForm/TextForm.js | 4 +- .../editor/TextForm/TextInput.js | 54 ++++++++++++------- src/components/distributions/editor/index.js | 6 +-- src/components/distributions/editor/style.css | 4 ++ .../guesstimator/formatter/formatters/lib.js | 5 +- 5 files changed, 45 insertions(+), 28 deletions(-) diff --git a/src/components/distributions/editor/TextForm/TextForm.js b/src/components/distributions/editor/TextForm/TextForm.js index f21a0fd10..693adb412 100644 --- a/src/components/distributions/editor/TextForm/TextForm.js +++ b/src/components/distributions/editor/TextForm/TextForm.js @@ -42,7 +42,7 @@ export class TextForm extends Component{ guesstimate: {input, guesstimateType}, onEscape, size, - hasErrors, + errors, onChangeInput, onAddData, onChangeGuesstimateType, @@ -66,7 +66,7 @@ export class TextForm extends Component{ onBlur={this._handleBlur.bind(this)} onChangeData={onAddData} ref='TextInput' - hasErrors={hasErrors} + errors={errors} width={shouldBeWide ? 'NARROW' : "WIDE"} /> diff --git a/src/components/distributions/editor/TextForm/TextInput.js b/src/components/distributions/editor/TextForm/TextInput.js index eb74146c4..08820f321 100644 --- a/src/components/distributions/editor/TextForm/TextInput.js +++ b/src/components/distributions/editor/TextForm/TextInput.js @@ -2,6 +2,7 @@ import React, {Component, PropTypes} from 'react' import $ from 'jquery' import {EditorState, Editor, ContentState, Modifier, CompositeDecorator} from 'draft-js' +import ReactTooltip from 'react-tooltip' import {isData, formatData} from 'lib/guesstimator/formatter/formatters/Data' import {getFactParams, addText, addSuggestionToEditorState, STATIC_DECORATOR, STATIC_DECORATOR_LIST} from 'lib/factParser' @@ -80,27 +81,40 @@ export default class TextInput extends Component{ } render() { - const [{hasErrors, width, value}, {editorState}] = [this.props, this.state] - const className = `TextInput ${width}` + (_.isEmpty(value) && hasErrors ? ' hasErrors' : '') + const ReactTooltipParams = {class: 'small-tooltip', delayShow: 0, delayHide: 0, place: 'bottom', effect: 'solid'} + const [{errors, width, value}, {editorState}] = [this.props, this.state] + const hasErrors = !_.isEmpty(errors) + const className = `TextInput ${width}` + (!_.isEmpty(value) && hasErrors ? ' hasErrors' : '') return ( - {e.stopPropagation()}} - onFocus={this.handleFocus.bind(this)} - > - this.props.onReturn(e.shiftKey)} - onTab={this.handleTab.bind(this)} - onBlur={this.handleBlur.bind(this)} - onChange={this.onChange.bind(this)} - ref='editor' - placeholder={'value'} - /> - +
+ {hasErrors && + +
    + {_.map(errors, e =>
  • {e}
    • )} +
+ + } + {e.stopPropagation()}} + onFocus={this.handleFocus.bind(this)} + data-tip + data-for='errors' + > + this.props.onReturn(e.shiftKey)} + onTab={this.handleTab.bind(this)} + onBlur={this.handleBlur.bind(this)} + onChange={this.onChange.bind(this)} + ref='editor' + placeholder={'value'} + /> + +
) } } diff --git a/src/components/distributions/editor/index.js b/src/components/distributions/editor/index.js index c8db2cd1a..64f297f05 100644 --- a/src/components/distributions/editor/index.js +++ b/src/components/distributions/editor/index.js @@ -91,11 +91,9 @@ export default class Guesstimate extends Component{ const {size, guesstimate, onOpen, errors} = this.props if(guesstimate.metric !== this.props.metricId) { return false } - const isLarge = (size === 'large') const hasData = !!guesstimate.data + const formClasses = `Guesstimate${size === 'large' ? ' large' : ''}` - let formClasses = 'Guesstimate' - formClasses += isLarge ? ' large' : '' return (
@@ -120,7 +118,7 @@ export default class Guesstimate extends Component{ onReturn={this.handleReturn.bind(this)} onTab={this.handleTab.bind(this)} size={size} - hasErrors={!_.isEmpty(errors)} + errors={errors} ref='TextForm' /> } diff --git a/src/components/distributions/editor/style.css b/src/components/distributions/editor/style.css index 7e8858dbc..058b1b370 100644 --- a/src/components/distributions/editor/style.css +++ b/src/components/distributions/editor/style.css @@ -4,6 +4,10 @@ @mixin small-input; } +.Guesstimate .small-tooltip { + /* TODO: Style */ +} + .Guesstimate .TextInput { line-height: 1.3em; font-size: 0.95em; diff --git a/src/lib/guesstimator/formatter/formatters/lib.js b/src/lib/guesstimator/formatter/formatters/lib.js index e8227c6a9..66bb51f47 100644 --- a/src/lib/guesstimator/formatter/formatters/lib.js +++ b/src/lib/guesstimator/formatter/formatters/lib.js @@ -75,11 +75,12 @@ export const confidenceIntervalTextMixin = Object.assign( } }, _matchesText(text) { return this._hasRelevantSymbol(text) }, - _confidenceIntervalTextErrors(text) { + _confidenceIntervalTextErrors(rawText) { + const text = rawText.trim() if (this._inputSymbols(text).length > 1) { return ['Must contain only 1 symbol'] } const numbers = this._numbers(text) - if (numbers.length !== 2) { return ['Must contain 2 inputs'] } + if (numbers.length !== 2) { return ['Must contain 2 numbers'] } if (!_.every(numbers, (e) => isParseableNumber(e))) { return ['Not all numbers are parseable'] } if (numbers[0] >= numbers[1]) { return ['Low number must be first'] } From 47743005ff49b0a415723eb830fd34e6d7a493c7 Mon Sep 17 00:00:00 2001 From: Matthew McDermott Date: Wed, 13 Jul 2016 10:13:27 -0700 Subject: [PATCH 02/12] Fixing up styling a bit. --- src/components/distributions/editor/TextForm/TextInput.js | 8 ++------ src/components/distributions/editor/style.css | 8 ++++++-- src/components/spaces/show/Toolbar/index.js | 2 +- src/components/spaces/show/style.css | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/components/distributions/editor/TextForm/TextInput.js b/src/components/distributions/editor/TextForm/TextInput.js index 08820f321..72ccc8ee4 100644 --- a/src/components/distributions/editor/TextForm/TextInput.js +++ b/src/components/distributions/editor/TextForm/TextInput.js @@ -81,18 +81,14 @@ export default class TextInput extends Component{ } render() { - const ReactTooltipParams = {class: 'small-tooltip', delayShow: 0, delayHide: 0, place: 'bottom', effect: 'solid'} + const ReactTooltipParams = {class: 'metric-errors-tooltip', delayShow: 0, delayHide: 0, type: 'error', place: 'bottom', effect: 'solid'} const [{errors, width, value}, {editorState}] = [this.props, this.state] const hasErrors = !_.isEmpty(errors) const className = `TextInput ${width}` + (!_.isEmpty(value) && hasErrors ? ' hasErrors' : '') return (
{hasErrors && - -
    - {_.map(errors, e =>
  • {e}
    • )} -
- + {errors[0]} } Viewing ) if (editableByMe && editsAllowed) { diff --git a/src/components/spaces/show/style.css b/src/components/spaces/show/style.css index 2994589f7..bbf13a725 100644 --- a/src/components/spaces/show/style.css +++ b/src/components/spaces/show/style.css @@ -197,7 +197,7 @@ min-height: 30em; } -.small-tooltip.small-tooltip.small-tooltip { +.header-action-tooltip.header-action-tooltip.header-action-tooltip { background: rgba(60, 64, 68, 0.95); opacity: 1; padding: 4px 13px; From 7cb9bff90df344aed9cb996b60cfbeef12f8a771 Mon Sep 17 00:00:00 2001 From: Matthew McDermott Date: Wed, 13 Jul 2016 13:40:06 -0700 Subject: [PATCH 03/12] Played briefly with error tooltip more. Couldn't fix z-index, so moved it to display only left for now to handle the issue. --- src/components/distributions/editor/TextForm/TextInput.js | 2 +- src/components/distributions/editor/style.css | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/distributions/editor/TextForm/TextInput.js b/src/components/distributions/editor/TextForm/TextInput.js index 72ccc8ee4..f76b7f9fc 100644 --- a/src/components/distributions/editor/TextForm/TextInput.js +++ b/src/components/distributions/editor/TextForm/TextInput.js @@ -81,7 +81,7 @@ export default class TextInput extends Component{ } render() { - const ReactTooltipParams = {class: 'metric-errors-tooltip', delayShow: 0, delayHide: 0, type: 'error', place: 'bottom', effect: 'solid'} + const ReactTooltipParams = {class: 'metric-errors-tooltip', delayShow: 0, delayHide: 0, type: 'error', place: 'left', effect: 'solid'} const [{errors, width, value}, {editorState}] = [this.props, this.state] const hasErrors = !_.isEmpty(errors) const className = `TextInput ${width}` + (!_.isEmpty(value) && hasErrors ? ' hasErrors' : '') diff --git a/src/components/distributions/editor/style.css b/src/components/distributions/editor/style.css index 223860acd..8b7a4177c 100644 --- a/src/components/distributions/editor/style.css +++ b/src/components/distributions/editor/style.css @@ -10,6 +10,7 @@ padding: 4px 13px; border-radius: 2px; transition: none; + z-index: 10000000 !important; } .Guesstimate .TextInput { From 068e4765261dff11c687bb34eeaf70cfcb91b794 Mon Sep 17 00:00:00 2001 From: Matthew McDermott Date: Wed, 13 Jul 2016 15:21:59 -0700 Subject: [PATCH 04/12] WIP; logging. --- src/components/metrics/card/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/metrics/card/index.js b/src/components/metrics/card/index.js index deb02a581..07faa7bb9 100644 --- a/src/components/metrics/card/index.js +++ b/src/components/metrics/card/index.js @@ -207,6 +207,7 @@ export default class MetricCard extends Component { _errors() { if (this.props.isTitle){ return [] } let errors = _.get(this.props.metric, 'simulation.sample.errors') + if (!_.isEmpty(errors)) { console.log(errors) } // TODO(matthew): Remove post testing. return errors ? errors.filter(e => !!e) : [] } From ea4c9a5d6689f98cfb703ca5ae37fcb51c020225 Mon Sep 17 00:00:00 2001 From: Matthew McDermott Date: Wed, 13 Jul 2016 17:47:46 -0700 Subject: [PATCH 05/12] Updating for merge. --- src/components/distributions/editor/TextForm/TextInput.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/distributions/editor/TextForm/TextInput.js b/src/components/distributions/editor/TextForm/TextInput.js index f76b7f9fc..8d89dff78 100644 --- a/src/components/distributions/editor/TextForm/TextInput.js +++ b/src/components/distributions/editor/TextForm/TextInput.js @@ -88,7 +88,7 @@ export default class TextInput extends Component{ return (
{hasErrors && - {errors[0]} + {errors[0].message} } Date: Wed, 13 Jul 2016 17:48:59 -0700 Subject: [PATCH 06/12] Bug fix that didn't trip hot reload --- src/lib/engine/guesstimate.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/engine/guesstimate.js b/src/lib/engine/guesstimate.js index b91d6dcb0..8fab1c47a 100644 --- a/src/lib/engine/guesstimate.js +++ b/src/lib/engine/guesstimate.js @@ -19,7 +19,7 @@ export function sample(guesstimate: Guesstimate, dGraph: DGraph, n: number = 1) let errors = [] const [parseError, item] = Guesstimator.parse(guesstimate) - errors.push(parseError) + if (!_.isEmpty(parseError)) {errors.push(parseError)} const [inputs, inputErrors] = item.needsExternalInputs() ? _inputMetricsWithValues(guesstimate, dGraph) : [{}, []] errors.push(...inputErrors) From 5de571969ea83aaa6bba2df3a29b32e414e99704 Mon Sep 17 00:00:00 2001 From: Matthew McDermott Date: Wed, 13 Jul 2016 18:09:58 -0700 Subject: [PATCH 07/12] Improved error messages. Error messages now flicker, however. --- .../metrics/card/MetricCardViewSection/index.js | 6 ++++-- src/lib/engine/guesstimate.js | 6 +++--- src/lib/errors/modelErrors.js | 3 ++- .../simulator-worker/simulator/evaluator.js | 16 +++++++++++----- src/lib/propagation/metric-propagation.js | 4 ++-- 5 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/components/metrics/card/MetricCardViewSection/index.js b/src/components/metrics/card/MetricCardViewSection/index.js index 4b49cc2c2..35f370f65 100644 --- a/src/components/metrics/card/MetricCardViewSection/index.js +++ b/src/components/metrics/card/MetricCardViewSection/index.js @@ -9,10 +9,12 @@ import StatTable from 'gComponents/simulations/stat_table/index' import {MetricToken} from 'gComponents/metrics/card/token/index' import SensitivitySection from 'gComponents/metrics/card/SensitivitySection/SensitivitySection' +import {INFINITE_LOOP_ERROR, INPUT_ERROR} from 'lib/errors/modelErrors' + import './style.css' -const isBreak = (errors) => {return errors[0] && (errors[0] === 'BROKEN_UPSTREAM' || errors[0] === 'BROKEN_INPUT' )} -const isInfiniteLoop = (errors) => {return errors[0] && (errors[0] === 'INFINITE_LOOP')} +const isBreak = (errors) => _.some(errors, e => e.type === INPUT_ERROR) +const isInfiniteLoop = (errors) => _.some(errors, e => e.type === INFINITE_LOOP_ERROR) // We have to display this section after it disappears // to ensure that the metric card gets selected after click. diff --git a/src/lib/engine/guesstimate.js b/src/lib/engine/guesstimate.js index 8fab1c47a..3e678a62c 100644 --- a/src/lib/engine/guesstimate.js +++ b/src/lib/engine/guesstimate.js @@ -2,7 +2,7 @@ import type {Guesstimate, Distribution, DGraph, Graph, Simulation} from './types' import {Guesstimator} from '../guesstimator/index' -import {GRAPH_ERROR} from 'lib/errors/modelErrors' +import {INPUT_ERROR} from 'lib/errors/modelErrors' export function equals(l, r) { return ( @@ -65,8 +65,8 @@ function _inputMetricsWithValues(guesstimate: Guesstimate, dGraph: DGraph): Obje const inputErrors = _.get(m, 'simulation.sample.errors') || [] errors.push(...inputErrors.map( ({type, message}) => { - if (type === GRAPH_ERROR) { return {type, message: message.replace('Broken input', 'Broken upstream input')} } - else { return {type: GRAPH_ERROR, message: `Broken input ${m.readableId}`} } + if (type === INPUT_ERROR) { return {type, message: message.replace('Broken input', 'Broken upstream input')} } + else { return {type: INPUT_ERROR, message: `Broken input ${m.readableId}`} } } )) }) diff --git a/src/lib/errors/modelErrors.js b/src/lib/errors/modelErrors.js index 31cd74a7c..6e357f44b 100644 --- a/src/lib/errors/modelErrors.js +++ b/src/lib/errors/modelErrors.js @@ -2,4 +2,5 @@ export const INTERNAL_ERROR = 'INTERNAL_ERROR', PARSER_ERROR = 'PARSER_ERROR', MATH_ERROR = 'MATH_ERROR', - GRAPH_ERROR = 'GRAPH_ERROR' + INPUT_ERROR = 'INPUT_ERROR', + INFINITE_LOOP_ERROR = 'INFINITE_LOOP_ERROR' diff --git a/src/lib/guesstimator/samplers/simulator-worker/simulator/evaluator.js b/src/lib/guesstimator/samplers/simulator-worker/simulator/evaluator.js index 182b50dfe..2b3106880 100644 --- a/src/lib/guesstimator/samplers/simulator-worker/simulator/evaluator.js +++ b/src/lib/guesstimator/samplers/simulator-worker/simulator/evaluator.js @@ -2,7 +2,7 @@ var _ = require('lodash') import math from 'mathjs' import {Distributions} from './distributions/distributions' import {ImpureConstructs} from './constructs/constructs' -import {MATH_ERROR} from 'lib/errors/modelErrors' +import {MATH_ERROR, PARSER_ERROR} from 'lib/errors/modelErrors' var Finance = require('financejs') const finance = new Finance() @@ -52,16 +52,22 @@ function sampleInputs(inputs, i) { function evaluate(compiled, inputs, n){ let values = [] + let errors = [] for (let i = 0; i < n; i++) { - const sampledInputs = sampleInputs(inputs,i) - const newSample = compiled.eval(sampledInputs) + const newSample = compiled.eval(sampleInputs(inputs,i)) if (_.isFinite(newSample)) { - values = values.concat(newSample) + values.push(newSample) + } else if ([Infinity, -Infinity].includes(newSample)) { + errors.push({type: MATH_ERROR, message: 'Divide by zero error'}) + values.push(newSample) + } else if (newSample.constructor.name === 'Unit') { + return {values, errors: [{type: PARSER_ERROR, message: "Functions can't contain units or suffixes"}]} } else { + if (__DEV__) { console.warn('Unidentified sample detected: ', newSample) } return {values, errors: [{type: MATH_ERROR, message: 'Sampling error detected'}]} } } - return {values} + return {values, errors: _.uniq(errors)} } diff --git a/src/lib/propagation/metric-propagation.js b/src/lib/propagation/metric-propagation.js index e0461275b..0a767c6b4 100644 --- a/src/lib/propagation/metric-propagation.js +++ b/src/lib/propagation/metric-propagation.js @@ -7,7 +7,7 @@ import type {Simulation, Graph} from '../lib/engine/types' import {addSimulation} from 'gModules/simulations/actions' import Simulator from './simulator' -import {GRAPH_ERROR, INTERNAL_ERROR} from 'lib/errors/modelErrors' +import {INFINITE_LOOP_ERROR, INTERNAL_ERROR} from 'lib/errors/modelErrors' function isRecentPropagation(propagationId: number, simulation: Simulation) { return !_.has(simulation, 'propagationId') || (propagationId >= simulation.propagationId) @@ -58,7 +58,7 @@ export default class MetricPropagation { propagationId: this.propagationId, sample: { values: [], - errors: [{type: GRAPH_ERROR, message: 'Infinite loop detected'}], + errors: [{type: INFINITE_LOOP_ERROR, message: 'Infinite loop detected'}], } } this._dispatch(dispatch, simulation) From 061851a8bdf5d572f1fb0f2b5f8a905c1af57f71 Mon Sep 17 00:00:00 2001 From: Matthew McDermott Date: Wed, 13 Jul 2016 23:11:36 -0700 Subject: [PATCH 08/12] Readable Error Explanations. --- .../editor/TextForm/TextInput.js | 2 +- src/lib/engine/dgraph.js | 13 +++++----- src/lib/engine/graph.js | 25 ++++++++++++------- src/lib/engine/guesstimate.js | 1 + .../simulator-worker/simulator/evaluator.js | 6 +++-- src/lib/propagation/graph-propagation.js | 8 ++---- 6 files changed, 31 insertions(+), 24 deletions(-) diff --git a/src/components/distributions/editor/TextForm/TextInput.js b/src/components/distributions/editor/TextForm/TextInput.js index 8d89dff78..5e53f38a4 100644 --- a/src/components/distributions/editor/TextForm/TextInput.js +++ b/src/components/distributions/editor/TextForm/TextInput.js @@ -46,7 +46,7 @@ export default class TextInput extends Component{ } this.setState(newState) - const text = newState.editorState.getCurrentContent().getPlainText('') + const text = newState.editorState.getCurrentContent().getPlainText('').trim() if (text === this.props.value) { return } if (isData(text)) { this.props.onChangeData(formatData(text)) diff --git a/src/lib/engine/dgraph.js b/src/lib/engine/dgraph.js index 7ee1b7fc3..5f2c8fa4c 100644 --- a/src/lib/engine/dgraph.js +++ b/src/lib/engine/dgraph.js @@ -1,16 +1,17 @@ /* @flow */ -import * as graph from './graph'; -import * as _guesstimate from './guesstimate'; -import type {DGraph, Sample} from './types.js' +import * as graph from './graph' +import * as _guesstimate from './guesstimate' +import type {DGraph, Sample} from './types' +import {INTERNAL_ERROR} from 'lib/errors/modelErrors' //borrowing a function from the graph library -const metric = graph.metric; +const metric = graph.metric export function runSimulation(dGraph:DGraph, metricId:string, n:number) { - const m = metric(dGraph, metricId); + const m = metric(dGraph, metricId) if (!m) { console.warn('Unknown metric referenced') - return Promise.resolve({errors: ['Unknown metric referenced']}) + return Promise.resolve({errors: [{type: INTERNAL_ERROR, message: 'Unknown metric referenced'}]}) } return _guesstimate.sample(m.guesstimate, dGraph, n) } diff --git a/src/lib/engine/graph.js b/src/lib/engine/graph.js index c719a8564..c9310cfed 100644 --- a/src/lib/engine/graph.js +++ b/src/lib/engine/graph.js @@ -1,15 +1,17 @@ -import * as _metric from './metric'; -import * as _dgraph from './dgraph'; -import * as _space from './space'; -import BasicGraph from '../basic_graph/basic-graph.js' +import * as _metric from './metric' +import * as _dgraph from './dgraph' +import * as _space from './space' + +import BasicGraph from 'lib/basic_graph/basic-graph' +import {INFINITE_LOOP_ERROR} from 'lib/errors/modelErrors' export function create(graphAttributes){ - return _.pick(graphAttributes, ['metrics', 'guesstimates', 'simulations']); + return _.pick(graphAttributes, ['metrics', 'guesstimates', 'simulations']) } export function denormalize(graph){ - let metrics = _.map(graph.metrics, m => _metric.denormalize(m, graph)); - return {metrics}; + let metrics = _.map(graph.metrics, m => _metric.denormalize(m, graph)) + return {metrics} } export function runSimulation(graph, metricId, n) { @@ -17,7 +19,7 @@ export function runSimulation(graph, metricId, n) { } export function metric(graph, id){ - return graph.metrics.find(m => (m.id === id)); + return graph.metrics.find(m => (m.id === id)) } function basicGraph(graph){ @@ -36,7 +38,12 @@ export function dependencyList(graph, spaceId) { export function dependencyTree(oGraph, graphFilters) { const {spaceId, metricId, onlyHead, notHead, onlyUnsimulated} = graphFilters - if (onlyHead) { return [[metricId, 0]] } + if (onlyHead) { + const existingErrors = _.get(oGraph.simulations.find(s => s.metric === metricId), 'sample.errors') + // This is a hack to prevent the error type from changing while editing metrics with infinite loops. + // TODO(matthew): Store denormalized check so this hack is not necessary. + if (!_.some(existingErrors, e => e.type === INFINITE_LOOP_ERROR)) { return [[metricId, 0]] } + } let graph = oGraph if (spaceId) { graph = _space.subset(oGraph, spaceId) } diff --git a/src/lib/engine/guesstimate.js b/src/lib/engine/guesstimate.js index 3e678a62c..1e20a0f05 100644 --- a/src/lib/engine/guesstimate.js +++ b/src/lib/engine/guesstimate.js @@ -65,6 +65,7 @@ function _inputMetricsWithValues(guesstimate: Guesstimate, dGraph: DGraph): Obje const inputErrors = _.get(m, 'simulation.sample.errors') || [] errors.push(...inputErrors.map( ({type, message}) => { + console.log(type, message, m.readableId) if (type === INPUT_ERROR) { return {type, message: message.replace('Broken input', 'Broken upstream input')} } else { return {type: INPUT_ERROR, message: `Broken input ${m.readableId}`} } } diff --git a/src/lib/guesstimator/samplers/simulator-worker/simulator/evaluator.js b/src/lib/guesstimator/samplers/simulator-worker/simulator/evaluator.js index 2b3106880..1980fce66 100644 --- a/src/lib/guesstimator/samplers/simulator-worker/simulator/evaluator.js +++ b/src/lib/guesstimator/samplers/simulator-worker/simulator/evaluator.js @@ -62,10 +62,12 @@ function evaluate(compiled, inputs, n){ errors.push({type: MATH_ERROR, message: 'Divide by zero error'}) values.push(newSample) } else if (newSample.constructor.name === 'Unit') { - return {values, errors: [{type: PARSER_ERROR, message: "Functions can't contain units or suffixes"}]} + return {values: [], errors: [{type: PARSER_ERROR, message: "Functions can't contain units or suffixes"}]} + } else if (typeof newSample === 'function') { + return {values: [], errors: [{type: PARSER_ERROR, message: "Incomplete function in input"}]} } else { if (__DEV__) { console.warn('Unidentified sample detected: ', newSample) } - return {values, errors: [{type: MATH_ERROR, message: 'Sampling error detected'}]} + return {values: [], errors: [{type: MATH_ERROR, message: 'Sampling error detected'}]} } } diff --git a/src/lib/propagation/graph-propagation.js b/src/lib/propagation/graph-propagation.js index c6577cf83..ea809b573 100644 --- a/src/lib/propagation/graph-propagation.js +++ b/src/lib/propagation/graph-propagation.js @@ -55,11 +55,7 @@ export class GraphPropagation { _step() { const i = (this.currentStep % this.orderedMetricIds.length) - return this._simulateMetric(this.orderedMetricPropagations[i]).then(() => {this.currentStep++}) - } - - _simulateMetric(metricPropagation) { - return metricPropagation.step(this._graph(), this.dispatch) + return this.orderedMetricPropagations[i].step(this._graph(), this.dispatch).then(() => {this.currentStep++}) } _graph(): Graph { @@ -71,7 +67,7 @@ export class GraphPropagation { _orderedMetricIdsAndErrors(graphFilters: object): Array { this.dependencies = e.graph.dependencyTree(this._graph(), graphFilters) - const orderedMetrics = _.sortBy(this.dependencies, function(n){return n[1]}).map(e => ({ + const orderedMetrics = _.sortBy(this.dependencies, n => n[1]).map(e => ({ id: e[0], errors: { inInfiniteLoop: !_.isFinite(e[1]) From 326e26900616a92dfae12889d840d13d37335216 Mon Sep 17 00:00:00 2001 From: Matthew McDermott Date: Thu, 14 Jul 2016 10:56:14 -0700 Subject: [PATCH 09/12] Adding error case for empty inputs. --- src/lib/engine/guesstimate.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/lib/engine/guesstimate.js b/src/lib/engine/guesstimate.js index 1e20a0f05..243ee3d17 100644 --- a/src/lib/engine/guesstimate.js +++ b/src/lib/engine/guesstimate.js @@ -62,12 +62,21 @@ function _inputMetricsWithValues(guesstimate: Guesstimate, dGraph: DGraph): Obje let errors = [] inputMetrics(guesstimate, dGraph).map(m => { inputs[m.readableId] = _.get(m, 'simulation.sample.values') + const inputErrors = _.get(m, 'simulation.sample.errors') || [] + if (_.isEmpty(inputs[m.readableId]) && _.isEmpty(inputErrors)) { + errors.push({type: INPUT_ERROR, message: `Empty input ${m.readableId}`}) + } + errors.push(...inputErrors.map( ({type, message}) => { - console.log(type, message, m.readableId) - if (type === INPUT_ERROR) { return {type, message: message.replace('Broken input', 'Broken upstream input')} } - else { return {type: INPUT_ERROR, message: `Broken input ${m.readableId}`} } + if (type === INPUT_ERROR) { + return { + type, + message: message.includes('upstream') ? message : message.replace('input', 'upstream input') + } + } + return {type: INPUT_ERROR, message: `Broken input ${m.readableId}`} } )) }) From 990b71e1dc4149f02a3a6fd86ee8ccdece3b9ee0 Mon Sep 17 00:00:00 2001 From: Matthew McDermott Date: Thu, 14 Jul 2016 11:06:31 -0700 Subject: [PATCH 10/12] Improving math errors a bit too. --- src/components/distributions/editor/TextForm/TextInput.js | 6 ++++-- .../samplers/simulator-worker/simulator/evaluator.js | 7 ++++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/components/distributions/editor/TextForm/TextInput.js b/src/components/distributions/editor/TextForm/TextInput.js index 5e53f38a4..4545cdbf2 100644 --- a/src/components/distributions/editor/TextForm/TextInput.js +++ b/src/components/distributions/editor/TextForm/TextInput.js @@ -6,6 +6,7 @@ import ReactTooltip from 'react-tooltip' import {isData, formatData} from 'lib/guesstimator/formatter/formatters/Data' import {getFactParams, addText, addSuggestionToEditorState, STATIC_DECORATOR, STATIC_DECORATOR_LIST} from 'lib/factParser' +import {INTERNAL_ERROR} from 'lib/errors/modelErrors' export default class TextInput extends Component{ displayName: 'Guesstimate-TextInput' @@ -85,10 +86,11 @@ export default class TextInput extends Component{ const [{errors, width, value}, {editorState}] = [this.props, this.state] const hasErrors = !_.isEmpty(errors) const className = `TextInput ${width}` + (!_.isEmpty(value) && hasErrors ? ' hasErrors' : '') + const displayedError = errors.find(e => e.type !== INTERNAL_ERROR) return (
- {hasErrors && - {errors[0].message} + {hasErrors && !!displayedError && + {displayedError.message} } Date: Fri, 15 Jul 2016 10:31:01 -0700 Subject: [PATCH 11/12] Fixed Tests. --- .../formatters/tests/DistributionNormalTextUpTo-test.js | 2 +- src/lib/guesstimator/index-test.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lib/guesstimator/formatter/formatters/tests/DistributionNormalTextUpTo-test.js b/src/lib/guesstimator/formatter/formatters/tests/DistributionNormalTextUpTo-test.js index c03c73a3d..21280aa1b 100644 --- a/src/lib/guesstimator/formatter/formatters/tests/DistributionNormalTextUpTo-test.js +++ b/src/lib/guesstimator/formatter/formatters/tests/DistributionNormalTextUpTo-test.js @@ -44,7 +44,7 @@ describe('DistributionTextUpTo', () => { examples.map(e => () => { it(`guesstimate ${JSON.stringify(e[0])} has errors ${JSON.stringify(e[1])}`, () => { - expect(formatter.errors(e[0]).length > 0).to.equal(e[1]) + expect(!_.isEmpty(formatter.error(e[0])).to.equal(e[1]) }) }).map(e => e()) }); diff --git a/src/lib/guesstimator/index-test.js b/src/lib/guesstimator/index-test.js index 841ecd75e..ea485be79 100644 --- a/src/lib/guesstimator/index-test.js +++ b/src/lib/guesstimator/index-test.js @@ -5,8 +5,8 @@ describe('Guesstimator', () => { describe('.parse', () => { it('works with a simple function', () => { const input = {text: '=34'} - const [errors, item] = Guesstimator.parse(input) - expect(errors.length).to.eq(0) + const [error, item] = Guesstimator.parse(input) + expect(_.isEmpty(error)).to.eq(true) const parsedInput = item.parsedInput expect(parsedInput.guesstimateType).to.eq('FUNCTION') @@ -23,7 +23,7 @@ describe('Guesstimator', () => { describe('#samplerType', () => { it('has many samplerTypes', () => { const input = {text: '=34'} - const [errors, item] = Guesstimator.parse(input) + const [error, item] = Guesstimator.parse(input) expect(item.samplerType().referenceName).to.eq('FUNCTION') }) }) From 177443fa7ee20fd536c170a8921a6f599664b828 Mon Sep 17 00:00:00 2001 From: Matthew McDermott Date: Fri, 15 Jul 2016 11:14:15 -0700 Subject: [PATCH 12/12] Fixing modal open state for error messaging. --- src/components/metrics/card/index.js | 3 ++- src/components/metrics/modal/index.js | 13 +++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/components/metrics/card/index.js b/src/components/metrics/card/index.js index 50203dfe0..1b54c4ce8 100644 --- a/src/components/metrics/card/index.js +++ b/src/components/metrics/card/index.js @@ -246,6 +246,7 @@ export default class MetricCard extends Component { {this.state.modalIsOpen && @@ -281,7 +282,7 @@ export default class MetricCard extends Component { onOpen={this.openModal.bind(this)} ref='DistributionEditor' size='small' - errors={errors} + errors={this.state.modalIsOpen ? [] : errors} onReturn={this.props.onReturn} onTab={this.props.onTab} /> diff --git a/src/components/metrics/modal/index.js b/src/components/metrics/modal/index.js index 7742d7d3e..677ec58c6 100644 --- a/src/components/metrics/modal/index.js +++ b/src/components/metrics/modal/index.js @@ -61,7 +61,7 @@ export class MetricModal extends Component { } const showSimulation = this.showSimulation() - const {closeModal, metric, onChangeGuesstimateDescription} = this.props + const {closeModal, metric, errors, onChangeGuesstimateDescription} = this.props const sortedSampleValues = _.get(metric, 'simulation.sample.sortedValues') const stats = _.get(metric, 'simulation.stats') const guesstimate = metric.guesstimate @@ -107,11 +107,12 @@ export class MetricModal extends Component {
- +