diff --git a/CHANGELOG.md b/CHANGELOG.md index e0580fd8b6..68ff61f920 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed bug when undoing irreversible operation. (#502) - Fixed minor issue with CHAR function logic. (#510) - Fixed `simpleCellAddressToString` behaviour when converting quoted sheet names. (#514) +- Fixed issues with numeric aggregation functions (#515) ## [0.1.3] - 2020-07-21 diff --git a/src/interpreter/ArithmeticHelper.ts b/src/interpreter/ArithmeticHelper.ts index d2d41df385..764f827195 100644 --- a/src/interpreter/ArithmeticHelper.ts +++ b/src/interpreter/ArithmeticHelper.ts @@ -311,118 +311,6 @@ export function divide(left: number, right: number): number | CellError { } } -/** - * Returns max from two numbers - * - * Implementation of max function which is used in interpreter. - * - * Errors are propagated, non-numerical values are neutral. - * - * @param left - left operand of addition - * @param right - right operand of addition - */ -export function max(left: InternalScalarValue, right: InternalScalarValue): InternalScalarValue { - if (left instanceof CellError) { - return left - } - if (right instanceof CellError) { - return right - } - if (typeof left === 'number') { - if (typeof right === 'number') { - return Math.max(left, right) - } else { - return left - } - } else if (typeof right === 'number') { - return right - } else { - return Number.NEGATIVE_INFINITY - } -} - -export function maxa(left: InternalScalarValue, right: InternalScalarValue): InternalScalarValue { - if (left instanceof CellError) { - return left - } - if (right instanceof CellError) { - return right - } - if (typeof left === 'boolean') { - left = coerceBooleanToNumber(left) - } - if (typeof right === 'boolean') { - right = coerceBooleanToNumber(right) - } - if (typeof left === 'number') { - if (typeof right === 'number') { - return Math.max(left, right) - } else { - return left - } - } else if (typeof right === 'number') { - return right - } else { - return Number.NEGATIVE_INFINITY - } -} - -/** - * Returns min from two numbers - * - * Implementation of min function which is used in interpreter. - * - * Errors are propagated, non-numerical values are neutral. - * - * @param left - left operand of addition - * @param right - right operand of addition - */ -export function min(left: InternalScalarValue, right: InternalScalarValue): InternalScalarValue { - if (left instanceof CellError) { - return left - } - if (right instanceof CellError) { - return right - } - if (typeof left === 'number') { - if (typeof right === 'number') { - return Math.min(left, right) - } else { - return left - } - } else if (typeof right === 'number') { - return right - } else { - return Number.POSITIVE_INFINITY - } -} - -export function mina(left: InternalScalarValue, right: InternalScalarValue): InternalScalarValue { - if (left instanceof CellError) { - return left - } - if (right instanceof CellError) { - return right - } - if (typeof left === 'boolean') { - left = coerceBooleanToNumber(left) - } - if (typeof right === 'boolean') { - right = coerceBooleanToNumber(right) - } - if (typeof left === 'number') { - if (typeof right === 'number') { - return Math.min(left, right) - } else { - return left - } - } else if (typeof right === 'number') { - return right - } else { - return Number.POSITIVE_INFINITY - } -} - export function numberCmp(left: number, right: number): number { if (left > right) { return 1 diff --git a/src/interpreter/plugin/CountBlankPlugin.ts b/src/interpreter/plugin/CountBlankPlugin.ts new file mode 100644 index 0000000000..ff918abd9a --- /dev/null +++ b/src/interpreter/plugin/CountBlankPlugin.ts @@ -0,0 +1,37 @@ +/** + * @license + * Copyright (c) 2020 Handsoncode. All rights reserved. + */ + +import {EmptyValue, InternalScalarValue, SimpleCellAddress} from '../../Cell' +import {ProcedureAst} from '../../parser' +import {ArgumentTypes, FunctionPlugin} from './FunctionPlugin' + +/** + * Interpreter plugin containing MEDIAN function + */ +export class CountBlankPlugin extends FunctionPlugin { + + public static implementedFunctions = { + 'COUNTBLANK': { + method: 'countblank', + parameters: [ + {argumentType: ArgumentTypes.SCALAR} + ], + repeatLastArgs: 1, + expandRanges: true, + }, + } + + public countblank(ast: ProcedureAst, formulaAddress: SimpleCellAddress): InternalScalarValue { + return this.runFunction(ast.args, formulaAddress, this.metadata('COUNTBLANK'), (...args: InternalScalarValue[]) => { + let counter = 0 + args.forEach((arg) => { + if(arg === EmptyValue) { + counter++ + } + }) + return counter + }) + } +} diff --git a/src/interpreter/plugin/MedianPlugin.ts b/src/interpreter/plugin/MedianPlugin.ts index 078daee3b8..4c0aa15f3e 100644 --- a/src/interpreter/plugin/MedianPlugin.ts +++ b/src/interpreter/plugin/MedianPlugin.ts @@ -5,7 +5,7 @@ import {CellError, ErrorType, InternalScalarValue, SimpleCellAddress} from '../../Cell' import {ErrorMessage} from '../../error-message' -import {ProcedureAst} from '../../parser' +import {AstNodeType, ProcedureAst} from '../../parser' import {ArgumentTypes, FunctionPlugin} from './FunctionPlugin' /** @@ -17,10 +17,10 @@ export class MedianPlugin extends FunctionPlugin { 'MEDIAN': { method: 'median', parameters: [ - {argumentType: ArgumentTypes.NOERROR}, - ], - repeatLastArgs: 1, - expandRanges: true, + {argumentType: ArgumentTypes.NOERROR}, + ], + repeatLastArgs: 1, + expandRanges: true, }, } @@ -35,6 +35,11 @@ export class MedianPlugin extends FunctionPlugin { public median(ast: ProcedureAst, formulaAddress: SimpleCellAddress): InternalScalarValue { return this.runFunction(ast.args, formulaAddress, this.metadata('MEDIAN'), (...args) => { const values: number[] = args.filter((val: InternalScalarValue) => (typeof val === 'number')) + ast.args.forEach((arg) => { //ugly but works + if (arg.type === AstNodeType.EMPTY) { + values.push(0) + } + }) if (values.length === 0) { return new CellError(ErrorType.NUM, ErrorMessage.OneValue) } diff --git a/src/interpreter/plugin/NumericAggregationPlugin.ts b/src/interpreter/plugin/NumericAggregationPlugin.ts index 896d52592e..20e6e36abf 100644 --- a/src/interpreter/plugin/NumericAggregationPlugin.ts +++ b/src/interpreter/plugin/NumericAggregationPlugin.ts @@ -9,27 +9,19 @@ import {CellError, EmptyValue, ErrorType, InternalScalarValue, SimpleCellAddress import {ErrorMessage} from '../../error-message' import {Maybe} from '../../Maybe' import {AstNodeType, CellRangeAst, ProcedureAst} from '../../parser' -import {coerceToRange, max, maxa, min, mina} from '../ArithmeticHelper' +import {coerceBooleanToNumber} from '../ArithmeticHelper' import {SimpleRangeValue} from '../InterpreterValue' import {FunctionPlugin} from './FunctionPlugin' import {ColumnRangeAst, RowRangeAst} from '../../parser/Ast' export type BinaryOperation = (left: T, right: T) => T -export type MapOperation = (arg: InternalScalarValue) => T +export type MapOperation = (arg: number) => T -function idMap(arg: InternalScalarValue): InternalScalarValue { - return arg -} +type coercionOperation = (arg: InternalScalarValue) => Maybe -function square(arg: InternalScalarValue): InternalScalarValue { - if (arg instanceof CellError) { - return arg - } else if (typeof arg === 'number') { - return arg * arg - } else { - return 0 - } +function identityMap(arg: number): number { + return arg } function zeroForInfinite(value: InternalScalarValue) { @@ -87,9 +79,6 @@ export class NumericAggregationPlugin extends FunctionPlugin { 'MINA': { method: 'mina', }, - 'COUNTBLANK': { - method: 'countblank', - }, 'COUNT': { method: 'count', }, @@ -113,41 +102,20 @@ export class NumericAggregationPlugin extends FunctionPlugin { * @param formulaAddress */ public sum(ast: ProcedureAst, formulaAddress: SimpleCellAddress): InternalScalarValue { - if (ast.args.some((ast) => ast.type === AstNodeType.EMPTY)) { - return new CellError(ErrorType.NUM, ErrorMessage.EmptyArg ) - } - return this.reduce(ast, formulaAddress, 0, 'SUM', this.interpreter.arithmeticHelper.nonstrictadd, idMap, (arg) => this.coerceScalarToNumberOrError(arg)) - } - - public sumsq(ast: ProcedureAst, formulaAddress: SimpleCellAddress): InternalScalarValue { if (ast.args.length < 1) { return new CellError(ErrorType.NA, ErrorMessage.WrongArgNumber) } - if (ast.args.some((ast) => ast.type === AstNodeType.EMPTY)) { - return new CellError(ErrorType.NUM, ErrorMessage.EmptyArg ) - } - return this.reduce(ast, formulaAddress, 0, 'SUMSQ', this.interpreter.arithmeticHelper.nonstrictadd, square, (arg) => this.coerceScalarToNumberOrError(arg)) + return this.reduce(ast, formulaAddress, 0, 'SUM', this.addWithEpsilon, identityMap, strictlyNumbers) } - public countblank(ast: ProcedureAst, formulaAddress: SimpleCellAddress): InternalScalarValue { + public sumsq(ast: ProcedureAst, formulaAddress: SimpleCellAddress): InternalScalarValue { if (ast.args.length < 1) { return new CellError(ErrorType.NA, ErrorMessage.WrongArgNumber) } - if (ast.args.some((ast) => ast.type === AstNodeType.EMPTY)) { - return new CellError(ErrorType.NUM, ErrorMessage.EmptyArg ) - } - let counter = 0 - for (const arg of ast.args) { - const rangeValue = coerceToRange(this.evaluateAst(arg, formulaAddress)) - for (const value of rangeValue.valuesFromTopLeftCorner()) { - if (value === EmptyValue) { - counter++ - } - } - } - return counter + return this.reduce(ast, formulaAddress, 0, 'SUMSQ', this.addWithEpsilon, (arg) => arg * arg, strictlyNumbers) } + /** * Corresponds to MAX(Number1, Number2, ...). * @@ -160,10 +128,9 @@ export class NumericAggregationPlugin extends FunctionPlugin { if (ast.args.length < 1) { return new CellError(ErrorType.NA, ErrorMessage.WrongArgNumber) } - if (ast.args.some((ast) => ast.type === AstNodeType.EMPTY)) { - return new CellError(ErrorType.NUM, ErrorMessage.EmptyArg ) - } - const value = this.reduce(ast, formulaAddress, Number.NEGATIVE_INFINITY, 'MAX', max, idMap, (arg) => this.coerceScalarToNumberOrError(arg)) + const value = this.reduce(ast, formulaAddress, Number.NEGATIVE_INFINITY, 'MAX', + (left, right) => Math.max(left, right), + identityMap, strictlyNumbers) return zeroForInfinite(value) } @@ -172,10 +139,9 @@ export class NumericAggregationPlugin extends FunctionPlugin { if (ast.args.length < 1) { return new CellError(ErrorType.NA, ErrorMessage.WrongArgNumber) } - if (ast.args.some((ast) => ast.type === AstNodeType.EMPTY)) { - return new CellError(ErrorType.NUM, ErrorMessage.EmptyArg ) - } - const value = this.reduce(ast, formulaAddress, Number.NEGATIVE_INFINITY, 'MAXA', maxa, idMap, (arg) => this.coerceScalarToNumberOrError(arg)) + const value = this.reduce(ast, formulaAddress, Number.NEGATIVE_INFINITY, 'MAXA', + (left, right) => Math.max(left, right), + identityMap, numbersBooleans) return zeroForInfinite(value) } @@ -192,10 +158,9 @@ export class NumericAggregationPlugin extends FunctionPlugin { if (ast.args.length < 1) { return new CellError(ErrorType.NA, ErrorMessage.WrongArgNumber) } - if (ast.args.some((ast) => ast.type === AstNodeType.EMPTY)) { - return new CellError(ErrorType.NUM, ErrorMessage.EmptyArg ) - } - const value = this.reduce(ast, formulaAddress, Number.POSITIVE_INFINITY, 'MIN', min, idMap, (arg) => this.coerceScalarToNumberOrError(arg)) + const value = this.reduce(ast, formulaAddress, Number.POSITIVE_INFINITY, 'MIN', + (left, right) => Math.min(left, right), + identityMap, strictlyNumbers) return zeroForInfinite(value) } @@ -204,10 +169,9 @@ export class NumericAggregationPlugin extends FunctionPlugin { if (ast.args.length < 1) { return new CellError(ErrorType.NA, ErrorMessage.WrongArgNumber) } - if (ast.args.some((ast) => ast.type === AstNodeType.EMPTY)) { - return new CellError(ErrorType.NUM, ErrorMessage.EmptyArg ) - } - const value = this.reduce(ast, formulaAddress, Number.POSITIVE_INFINITY, 'MINA', mina, idMap, (arg) => this.coerceScalarToNumberOrError(arg)) + const value = this.reduce(ast, formulaAddress, Number.POSITIVE_INFINITY, 'MINA', + (left, right) => Math.min(left, right), + identityMap, numbersBooleans) return zeroForInfinite(value) } @@ -216,14 +180,10 @@ export class NumericAggregationPlugin extends FunctionPlugin { if (ast.args.length < 1) { return new CellError(ErrorType.NA, ErrorMessage.WrongArgNumber) } - if (ast.args.some((ast) => ast.type === AstNodeType.EMPTY)) { - return new CellError(ErrorType.NUM, ErrorMessage.EmptyArg ) - } - const value = this.reduce(ast, formulaAddress, 0, 'COUNT', (left, right) => { - return left + right - }, (arg): number => { - return (typeof arg === 'number') ? 1 : 0 - }, (arg) => this.coerceScalarToNumberOrError(arg)) + const value = this.reduce(ast, formulaAddress, 0, 'COUNT', + (left, right) => left + right, identityMap, + (arg) => (typeof arg === 'number') ? 1 : 0 + ) return value } @@ -232,14 +192,10 @@ export class NumericAggregationPlugin extends FunctionPlugin { if (ast.args.length < 1) { return new CellError(ErrorType.NA, ErrorMessage.WrongArgNumber) } - if (ast.args.some((ast) => ast.type === AstNodeType.EMPTY)) { - return new CellError(ErrorType.NUM, ErrorMessage.EmptyArg ) - } - const value = this.reduce(ast, formulaAddress, 0, 'COUNTA', (left, right) => { - return left + right - }, (arg): number => { - return (arg === EmptyValue) ? 0 : 1 - }, (arg) => this.coerceScalarToNumberOrError(arg)) + const value = this.reduce(ast, formulaAddress, 0, 'COUNTA', (left, right) => left + right, + identityMap, + (arg) => (arg === EmptyValue) ? 0 : 1 + ) return value } @@ -248,32 +204,18 @@ export class NumericAggregationPlugin extends FunctionPlugin { if (ast.args.length < 1) { return new CellError(ErrorType.NA, ErrorMessage.WrongArgNumber) } - if (ast.args.some((ast) => ast.type === AstNodeType.EMPTY)) { - return new CellError(ErrorType.NUM, ErrorMessage.EmptyArg ) - } - - const result = this.reduce(ast, formulaAddress, AverageResult.empty, 'AVERAGE', (left, right) => { - if (left instanceof CellError) { - return left - } else if (right instanceof CellError) { - return right - } else { + const result = this.reduce(ast, formulaAddress, AverageResult.empty, 'AVERAGE', (left, right) => { return left.compose(right) - } - }, (arg): AverageResult | CellError => { - if (arg instanceof CellError) { - return arg - } else if (typeof arg === 'number') { + }, (arg): AverageResult => { return AverageResult.single(arg) - } else { - return AverageResult.empty - } - }, (arg) => this.coerceScalarToNumberOrError(arg)) + }, + strictlyNumbers + ) if (result instanceof CellError) { return result } else { - return result.averageValue() || new CellError(ErrorType.DIV_BY_ZERO) + return result.averageValue() ?? new CellError(ErrorType.DIV_BY_ZERO) } } @@ -281,32 +223,11 @@ export class NumericAggregationPlugin extends FunctionPlugin { if (ast.args.length < 1) { return new CellError(ErrorType.NA, ErrorMessage.WrongArgNumber) } - if (ast.args.some((ast) => ast.type === AstNodeType.EMPTY)) { - return new CellError(ErrorType.NUM, ErrorMessage.EmptyArg ) - } - - const result = this.reduce(ast, formulaAddress, AverageResult.empty, 'AVERAGE', (left, right) => { - if (left instanceof CellError) { - return left - } else if (right instanceof CellError) { - return right - } else { - return left.compose(right) - } - }, (arg): AverageResult | CellError => { - if (arg === EmptyValue) { - return AverageResult.empty - } else if (arg instanceof CellError) { - return arg - } else { - const coercedArg = this.interpreter.arithmeticHelper.coerceNonDateScalarToMaybeNumber(arg) - if (coercedArg === undefined) { - return AverageResult.empty - } else { - return AverageResult.single(coercedArg) - } - } - }, (arg) => this.coerceScalarToNumberOrError(arg)) + const result = this.reduce(ast, formulaAddress, AverageResult.empty, 'AVERAGE', + (left, right) => left.compose(right), + (arg): AverageResult => AverageResult.single(arg), + numbersBooleans + ) if (result instanceof CellError) { return result @@ -315,6 +236,8 @@ export class NumericAggregationPlugin extends FunctionPlugin { } } + private addWithEpsilon = (left: number, right: number): number => this.interpreter.arithmeticHelper.addWithEpsilon(left, right) + /** * Reduces procedure arguments with given reducing function * @@ -326,44 +249,59 @@ export class NumericAggregationPlugin extends FunctionPlugin { * @param mapFunction * @param coerceFunction * */ - private reduce(ast: ProcedureAst, formulaAddress: SimpleCellAddress, initialAccValue: T, functionName: string, reducingFunction: BinaryOperation, mapFunction: MapOperation, coerceFunction: (arg: InternalScalarValue) => InternalScalarValue): T { - return ast.args.reduce((acc: T, arg) => { - let value + private reduce(ast: ProcedureAst, formulaAddress: SimpleCellAddress, initialAccValue: T, functionName: string, reducingFunction: BinaryOperation, mapFunction: MapOperation, coercionFunction: coercionOperation): CellError | T { + return ast.args.reduce((acc: T | CellError, arg) => { + if (acc instanceof CellError) { + return acc + } if (arg.type === AstNodeType.CELL_RANGE || arg.type === AstNodeType.COLUMN_RANGE || arg.type === AstNodeType.ROW_RANGE) { - value = this.evaluateRange(arg, formulaAddress, acc, functionName, reducingFunction, mapFunction) + return this.evaluateRange(arg, formulaAddress, acc, functionName, reducingFunction, mapFunction, coercionFunction) + } + let value + value = this.evaluateAst(arg, formulaAddress) + if (value instanceof SimpleRangeValue) { + const coercedRangeValues = Array.from(value.valuesFromTopLeftCorner()) + .map(coercionFunction) + .filter((arg) => (arg !== undefined)) as (CellError | number)[] + + return coercedRangeValues + .map((arg) => { + if (arg instanceof CellError) { + return arg + } else { + return mapFunction(arg) + } + }) + .reduce((left, right) => { + if (left instanceof CellError) { + return left + } else if (right instanceof CellError) { + return right + } else { + return reducingFunction(left, right) + } + }, acc) + } else if (arg.type === AstNodeType.CELL_REFERENCE) { + value = coercionFunction(value) + if (value === undefined) { + return acc + } } else { - value = this.evaluateAst(arg, formulaAddress) - if (value instanceof SimpleRangeValue) { - value = this.reduceRange(Array.from(value.valuesFromTopLeftCorner()).map(mapFunction), initialAccValue, reducingFunction) - } else if (arg.type === AstNodeType.CELL_REFERENCE) { - value = mapFunction(value) - value = reducingFunction(initialAccValue, value) - } else { - value = coerceFunction(value) - value = mapFunction(value) + value = this.coerceScalarToNumberOrError(value) + value = coercionFunction(value) + if (value === undefined) { + return acc } + } + if (value instanceof CellError) { + return value } - return reducingFunction(acc, value) + return reducingFunction(acc, mapFunction(value)) }, initialAccValue) } - /** - * Reduces list of cell values with given reducing function - * - * @param rangeValues - list of values to reduce - * @param initialAccValue - initial accumulator value for reducing function - * @param reducingFunction - reducing function - */ - private reduceRange(rangeValues: T[], initialAccValue: T, reducingFunction: BinaryOperation): T { - let acc = initialAccValue - for (const val of rangeValues) { - acc = reducingFunction(acc, val) - } - return acc - } - /** * Performs range operation on given range * @@ -373,13 +311,13 @@ export class NumericAggregationPlugin extends FunctionPlugin { * @param functionName - function name to use as cache key * @param reducingFunction - reducing function */ - private evaluateRange(ast: CellRangeAst | ColumnRangeAst | RowRangeAst, formulaAddress: SimpleCellAddress, initialAccValue: T, functionName: string, reducingFunction: BinaryOperation, mapFunction: MapOperation): T { + private evaluateRange(ast: CellRangeAst | ColumnRangeAst | RowRangeAst, formulaAddress: SimpleCellAddress, initialAccValue: T, functionName: string, reducingFunction: BinaryOperation, mapFunction: MapOperation, coercionFunction: coercionOperation): T | CellError { let range try { range = AbsoluteCellRange.fromAst(ast, formulaAddress) } catch (err) { if (err.message === DIFFERENT_SHEETS_ERROR) { - return mapFunction(new CellError(ErrorType.REF, ErrorMessage.RangeManySheets)) + return new CellError(ErrorType.REF, ErrorMessage.RangeManySheets) } else { throw err } @@ -390,10 +328,18 @@ export class NumericAggregationPlugin extends FunctionPlugin { const rangeVertex = this.dependencyGraph.getRange(rangeStart, rangeEnd)! assert.ok(rangeVertex, 'Range does not exists in graph') - let value = rangeVertex.getFunctionValue(functionName) as T + let value = rangeVertex.getFunctionValue(functionName) as (T | CellError) if (!value) { - const rangeValues = this.getRangeValues(functionName, range, mapFunction) - value = this.reduceRange(rangeValues, initialAccValue, reducingFunction) + const rangeValues = this.getRangeValues(functionName, range, mapFunction, coercionFunction) + value = rangeValues.reduce((arg1, arg2) => { + if (arg1 instanceof CellError) { + return arg1 + } else if (arg2 instanceof CellError) { + return arg2 + } else { + return reducingFunction(arg1, arg2) + } + }, initialAccValue) rangeVertex.setFunctionValue(functionName, value) } @@ -409,8 +355,8 @@ export class NumericAggregationPlugin extends FunctionPlugin { * @param functionName - function name (e.g. SUM) * @param range - cell range */ - private getRangeValues(functionName: string, range: AbsoluteCellRange, mapFunction: MapOperation): T[] { - const rangeResult: T[] = [] + private getRangeValues(functionName: string, range: AbsoluteCellRange, mapFunction: MapOperation, coercionFunction: coercionOperation): (T | CellError)[] { + const rangeResult: (T | CellError)[] = [] const {smallerRangeVertex, restRange} = this.dependencyGraph.rangeMapping.findSmallerRange(range) const currentRangeVertex = this.dependencyGraph.getRange(range.start, range.end)! let actualRange: AbsoluteCellRange @@ -420,7 +366,12 @@ export class NumericAggregationPlugin extends FunctionPlugin { rangeResult.push(cachedValue) } else { for (const cellFromRange of smallerRangeVertex.range.addresses(this.dependencyGraph)) { - rangeResult.push(mapFunction(this.dependencyGraph.getScalarValue(cellFromRange))) + const val = coercionFunction(this.dependencyGraph.getScalarValue(cellFromRange)) + if (val instanceof CellError) { + rangeResult.push(val) + } else if (val !== undefined) { + rangeResult.push(mapFunction(val)) + } } } actualRange = restRange @@ -428,9 +379,35 @@ export class NumericAggregationPlugin extends FunctionPlugin { actualRange = range } for (const cellFromRange of actualRange.addresses(this.dependencyGraph)) { - rangeResult.push(mapFunction(this.dependencyGraph.getScalarValue(cellFromRange))) + const val = coercionFunction(this.dependencyGraph.getScalarValue(cellFromRange)) + if (val instanceof CellError) { + rangeResult.push(val) + } else if (val !== undefined) { + rangeResult.push(mapFunction(val)) + } } return rangeResult } } + +function strictlyNumbers(arg: InternalScalarValue): Maybe { + if (typeof arg === 'number' || arg instanceof CellError) { + return arg + } else { + return undefined + } +} + +function numbersBooleans(arg: InternalScalarValue): Maybe { + if (typeof arg === 'boolean') { + return coerceBooleanToNumber(arg) + } else if (typeof arg === 'number' || arg instanceof CellError) { + return arg + } else if (typeof arg === 'string') { + return 0 + } else { + return undefined + } +} + diff --git a/src/interpreter/plugin/index.ts b/src/interpreter/plugin/index.ts index b26299efb9..0ebe186b79 100644 --- a/src/interpreter/plugin/index.ts +++ b/src/interpreter/plugin/index.ts @@ -38,3 +38,4 @@ export {CodePlugin} from './CodePlugin' export {ErrorFunctionPlugin} from './ErrorFunctionPlugin' export {CorrelPlugin} from './CorrelPlugin' export {FormulaTextPlugin} from './FormulaTextPlugin' +export {CountBlankPlugin} from './CountBlankPlugin' diff --git a/test/interpreter/aggregation-arguments.spec.ts b/test/interpreter/aggregation-arguments.spec.ts new file mode 100644 index 0000000000..ab10cbba1d --- /dev/null +++ b/test/interpreter/aggregation-arguments.spec.ts @@ -0,0 +1,74 @@ +import {HyperFormula} from '../../src' +import {adr} from '../testUtils' + +describe('AVERAGE function', () => { + it('should work for empty arg', () => { + const engine = HyperFormula.buildFromArray([ + ['=AVERAGE(1,)'], + ]) + + expect(engine.getCellValue(adr('A1'))).toEqual(0.5) + }) + + it('should work for empty reference', () => { + const engine = HyperFormula.buildFromArray([ + ['=AVERAGE(A2,B2)'], + [1, null] + ]) + + expect(engine.getCellValue(adr('A1'))).toEqual(1) + }) + + it('should work for range with empty val', () => { + const engine = HyperFormula.buildFromArray([ + ['=AVERAGE(A2:B2)'], + [1, null] + ]) + + expect(engine.getCellValue(adr('A1'))).toEqual(1) + }) + + it('should work for empty reference + empty arg', () => { + const engine = HyperFormula.buildFromArray([ + ['=AVERAGE(A2,B2,)'], + [1, null] + ]) + + expect(engine.getCellValue(adr('A1'))).toEqual(0.5) + }) + + it('should work for range with empty val + empty arg', () => { + const engine = HyperFormula.buildFromArray([ + ['=AVERAGE(A2:B2,)'], + [1, null] + ]) + + expect(engine.getCellValue(adr('A1'))).toEqual(0.5) + }) + + it('should work for coercible arg', () => { + const engine = HyperFormula.buildFromArray([ + ['=AVERAGE(2,TRUE())'], + ]) + + expect(engine.getCellValue(adr('A1'))).toEqual(1.5) + }) + + it('should work for coercible value in reference', () => { + const engine = HyperFormula.buildFromArray([ + ['=AVERAGE(A2,B2)'], + [2, true] + ]) + + expect(engine.getCellValue(adr('A1'))).toEqual(2) + }) + + it('should work for coercible value in range', () => { + const engine = HyperFormula.buildFromArray([ + ['=AVERAGE(A2:B2)'], + [2, true] + ]) + + expect(engine.getCellValue(adr('A1'))).toEqual(2) + }) +}) diff --git a/test/interpreter/function-averagea.spec.ts b/test/interpreter/function-averagea.spec.ts index a439d376bb..4dac5b511b 100644 --- a/test/interpreter/function-averagea.spec.ts +++ b/test/interpreter/function-averagea.spec.ts @@ -36,19 +36,20 @@ describe('AVERAGEA', () => { ['39', null, '=AVERAGEA(A3:B3)'], ]) - expect(engine.getCellValue(adr('C1'))).toEqual(20) + expect(engine.getCellValue(adr('C1'))).toEqual(19.5) expect(engine.getCellValue(adr('C2'))).toEqual(20) expect(engine.getCellValue(adr('C3'))).toEqual(39) }) it('error when no meaningful arguments', () => { const engine = HyperFormula.buildFromArray([ - ['foo'], - [null], - ['=AVERAGEA(A1:A2)'] + [null, 'foo'], + [null, null], + ['=AVERAGEA(A1:A2)', '=AVERAGEA(B1:B2)'] ]) expect(engine.getCellValue(adr('A3'))).toEqual(detailedError(ErrorType.DIV_BY_ZERO)) + expect(engine.getCellValue(adr('B3'))).toEqual(0) }) it('over a range value', () => { diff --git a/test/interpreter/function-count.spec.ts b/test/interpreter/function-count.spec.ts index 47cbf04956..65f02870e3 100644 --- a/test/interpreter/function-count.spec.ts +++ b/test/interpreter/function-count.spec.ts @@ -1,5 +1,4 @@ -import {HyperFormula} from '../../src' -import {ErrorType} from '../../src/Cell' +import {ErrorType, HyperFormula} from '../../src' import {ErrorMessage} from '../../src/error-message' import {adr, detailedError} from '../testUtils' @@ -38,7 +37,7 @@ describe('COUNT', () => { expect(engine.getCellValue(adr('A3'))).toEqual(4) }) - it('error ranges doesnt count', () => { + it('error in ranges', () => { const engine = HyperFormula.buildFromArray([ ['1', '2'], ['3', '4'], @@ -58,4 +57,19 @@ describe('COUNT', () => { expect(engine.getCellValue(adr('A3'))).toEqual(2) }) + + it('should work with explicit error in arg', () => { + const engine = HyperFormula.buildFromArray([ + ['=COUNT(NA())'], + ]) + expect(engine.getCellValue(adr('A1'))).toEqual(0) + }) + + it('should work for empty arg', () => { + const engine = HyperFormula.buildFromArray([ + ['=COUNT(1,)'], + ]) + + expect(engine.getCellValue(adr('A1'))).toEqual(2) + }) }) diff --git a/test/interpreter/function-counta.spec.ts b/test/interpreter/function-counta.spec.ts index 831b6ea914..4e47631d53 100644 --- a/test/interpreter/function-counta.spec.ts +++ b/test/interpreter/function-counta.spec.ts @@ -38,7 +38,7 @@ describe('COUNTA', () => { expect(engine.getCellValue(adr('A3'))).toEqual(4) }) - it('error ranges doesnt count', () => { + it('error in ranges', () => { const engine = HyperFormula.buildFromArray([ ['1', '2'], ['3', '4'], @@ -58,4 +58,19 @@ describe('COUNTA', () => { expect(engine.getCellValue(adr('A3'))).toEqual(4) }) + + it('should work with explicit error in arg', () => { + const engine = HyperFormula.buildFromArray([ + ['=COUNTA(NA())'], + ]) + expect(engine.getCellValue(adr('A1'))).toEqual(1) + }) + + it('should work for empty arg', () => { + const engine = HyperFormula.buildFromArray([ + ['=COUNTA(1,)'], + ]) + + expect(engine.getCellValue(adr('A1'))).toEqual(2) //Compatible with product 2 + }) }) diff --git a/test/interpreter/function-countblank.spec.ts b/test/interpreter/function-countblank.spec.ts index f176acfa75..66007be6da 100644 --- a/test/interpreter/function-countblank.spec.ts +++ b/test/interpreter/function-countblank.spec.ts @@ -5,8 +5,12 @@ import {adr, detailedError} from '../testUtils' describe('COUNTBLANK', () => { it('with empty args', () => { - const engine = HyperFormula.buildFromArray([['=COUNTBLANK()']]) + const engine = HyperFormula.buildFromArray([ + ['=COUNTBLANK()'], + ['=COUNTBLANK(,)'] + ]) expect(engine.getCellValue(adr('A1'))).toEqual(detailedError(ErrorType.NA, ErrorMessage.WrongArgNumber)) + expect(engine.getCellValue(adr('A2'))).toEqual(2) }) it('with args', () => { diff --git a/test/interpreter/function-maxa.spec.ts b/test/interpreter/function-maxa.spec.ts index e4a24c4faa..0044e969ea 100644 --- a/test/interpreter/function-maxa.spec.ts +++ b/test/interpreter/function-maxa.spec.ts @@ -38,7 +38,7 @@ describe('MAXA', () => { it('MAXA of strings and -1', () => { const engine = HyperFormula.buildFromArray([['foo'], ['bar'], ['-1'], ['=MAXA(A1:A3)']]) - expect(engine.getCellValue(adr('A4'))).toEqual(-1) + expect(engine.getCellValue(adr('A4'))).toEqual(0) }) it('MAXA of empty value', () => { @@ -47,8 +47,12 @@ describe('MAXA', () => { }) it('MAXA of empty value and some negative number', () => { - const engine = HyperFormula.buildFromArray([['', '-1', '=MAXA(A1,B1)']]) - expect(engine.getCellValue(adr('C1'))).toEqual(-1) + const engine = HyperFormula.buildFromArray([ + ['', '-1', '=MAXA(A1,B1)'], + [null, '-1', '=MAXA(A2,B2)'], + ]) + expect(engine.getCellValue(adr('C1'))).toEqual(0) + expect(engine.getCellValue(adr('C2'))).toEqual(-1) }) it('over a range value', () => { diff --git a/test/interpreter/function-median.spec.ts b/test/interpreter/function-median.spec.ts index 5f6d062bae..08db7619c0 100644 --- a/test/interpreter/function-median.spec.ts +++ b/test/interpreter/function-median.spec.ts @@ -95,4 +95,14 @@ describe('Function MEDIAN', () => { expect(engine.getCellValue(adr('A1'))).toEqual(detailedError(ErrorType.NUM, ErrorMessage.OneValue)) }) + + it('empty args as 0', () => { + const engine = HyperFormula.buildFromArray([ + ['=MEDIAN(1,2,3,,)'], + ['=MEDIAN(,)'] + ]) + + expect(engine.getCellValue(adr('A1'))).toEqual(1) + expect(engine.getCellValue(adr('A2'))).toEqual(0) + }) }) diff --git a/test/interpreter/function-mina.spec.ts b/test/interpreter/function-mina.spec.ts index 5a52e628db..3980d2623c 100644 --- a/test/interpreter/function-mina.spec.ts +++ b/test/interpreter/function-mina.spec.ts @@ -43,9 +43,13 @@ describe('MINA', () => { }) it('MINA of empty value and some positive number', () => { - const engine = HyperFormula.buildFromArray([['', '1', '=MINA(A1,B1)']]) + const engine = HyperFormula.buildFromArray([ + ['', '1', '=MINA(A1,B1)'], + [null, '1', '=MINA(A2,B2)'], + ]) - expect(engine.getCellValue(adr('C1'))).toEqual(1) + expect(engine.getCellValue(adr('C1'))).toEqual(0) + expect(engine.getCellValue(adr('C2'))).toEqual(1) }) it('over a range value', () => { diff --git a/test/interpreter/function-sum.spec.ts b/test/interpreter/function-sum.spec.ts index 6a45d79221..159e077989 100644 --- a/test/interpreter/function-sum.spec.ts +++ b/test/interpreter/function-sum.spec.ts @@ -1,5 +1,4 @@ -import {HyperFormula} from '../../src' -import {ErrorType} from '../../src/Cell' +import {ErrorType, HyperFormula} from '../../src' import {ErrorMessage} from '../../src/error-message' import {adr, detailedError} from '../testUtils' @@ -7,7 +6,7 @@ describe('SUM', () => { it('SUM without args', () => { const engine = HyperFormula.buildFromArray([['=SUM()']]) - expect(engine.getCellValue(adr('A1'))).toEqual(0) + expect(engine.getCellValue(adr('A1'))).toEqual(detailedError(ErrorType.NA, ErrorMessage.WrongArgNumber)) }) it('SUM with args', () => { diff --git a/test/interpreter/scalar.spec.ts b/test/interpreter/scalar.spec.ts index 661773b9c3..494034782a 100644 --- a/test/interpreter/scalar.spec.ts +++ b/test/interpreter/scalar.spec.ts @@ -1,7 +1,7 @@ import {ErrorType} from '../../src' import {Config} from '../../src/Config' import {DateTimeHelper} from '../../src/DateTimeHelper' -import {ArithmeticHelper, max, min} from '../../src/interpreter/ArithmeticHelper' +import {ArithmeticHelper} from '../../src/interpreter/ArithmeticHelper' import {NumberLiteralHelper} from '../../src/NumberLiteralHelper' import {CellError} from '../../src/Cell' @@ -33,48 +33,3 @@ describe('nonstrictadd', () => { }) }) -describe('max', () => { - it('max', () => { - expect(max(2, 3)).toEqual(3) - }) - - it('return error of right operand', () => { - expect(max(2, new CellError(ErrorType.DIV_BY_ZERO))).toEqual(new CellError(ErrorType.DIV_BY_ZERO)) - }) - - it('return error of left operand if both present', () => { - expect(max(new CellError(ErrorType.NA), new CellError(ErrorType.DIV_BY_ZERO))).toEqual(new CellError(ErrorType.NA)) - }) - - it('ignores non-numerics', () => { - expect(max('foo', 5)).toEqual(5) - expect(max(5, 'foo')).toEqual(5) - }) - - it('returns negative infinity if only non-numerics', () => { - expect(max('bar', 'foo')).toEqual(Number.NEGATIVE_INFINITY) - }) -}) - -describe('min', () => { - it('min', () => { - expect(min(2, 3)).toEqual(2) - }) - - it('return error of right operand', () => { - expect(min(2, new CellError(ErrorType.DIV_BY_ZERO))).toEqual(new CellError(ErrorType.DIV_BY_ZERO)) - }) - - it('return error of left operand if both present', () => { - expect(min(new CellError(ErrorType.NA), new CellError(ErrorType.DIV_BY_ZERO))).toEqual(new CellError(ErrorType.NA)) - }) - - it('ignores non-numerics', () => { - expect(min('foo', 5)).toEqual(5) - expect(min(-5, 'foo')).toEqual(-5) - }) - - it('returns positive infinity if only non-numerics', () => { - expect(min('bar', 'foo')).toEqual(Number.POSITIVE_INFINITY) - }) -}) diff --git a/test/optional-parameters.spec.ts b/test/optional-parameters.spec.ts index c5bd52320b..2c1cafda7f 100644 --- a/test/optional-parameters.spec.ts +++ b/test/optional-parameters.spec.ts @@ -61,7 +61,7 @@ describe('Nonexistent metadata', () => { ]) expect(engine.getCellValue(adr('A1'))).toEqual(1901) - expect(engine.getCellValue(adr('A2'))).toEqual(detailedError(ErrorType.NUM, ErrorMessage.EmptyArg )) //TODO when SUM() is fixed, it should evaluate to 1 + expect(engine.getCellValue(adr('A2'))).toEqual(1) expect(engine.getCellValue(adr('A3'))).toEqual('abcd') })