diff --git a/CHANGELOG.md b/CHANGELOG.md index 562e6feb3..7e535ebc2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Fixed +- Fixed the MAXPOOL and MEDIANPOOL functions throwing an uncaught `TypeError` instead of returning the `#VALUE!` error when the range dimensions are not a whole multiple of the window size and the stride. [#1718](https://github.com/handsontable/hyperformula/pull/1718) - Fixed the `MOD` function returning a remainder with the sign of the dividend instead of the sign of the divisor, which made the results differ from Excel and Google Sheets for arguments with opposite signs (e.g. `=MOD(-3, 12)` now returns `9` instead of `-3`). [#1747](https://github.com/handsontable/hyperformula/issues/1747) ## [3.4.0] - 2026-08-10 diff --git a/src/error-message.ts b/src/error-message.ts index 5e3afdbea..4b80ee06f 100644 --- a/src/error-message.ts +++ b/src/error-message.ts @@ -12,6 +12,7 @@ export class ErrorMessage { public static EmptyArg = 'Empty function argument.' public static EmptyArray = 'Empty array not allowed.' public static ArrayDimensions = 'Array dimensions are not compatible.' + public static PoolDimensions = 'Range dimensions are not compatible with the window size and the stride.' public static NoSpaceForArrayResult = 'No space for array result.' public static ValueSmall = 'Value too small.' public static ValueLarge = 'Value too large.' diff --git a/src/interpreter/functionMetadata/categories/matrix-functions.ts b/src/interpreter/functionMetadata/categories/matrix-functions.ts index 940c3b103..d4a9b73ad 100644 --- a/src/interpreter/functionMetadata/categories/matrix-functions.ts +++ b/src/interpreter/functionMetadata/categories/matrix-functions.ts @@ -12,15 +12,15 @@ import {FunctionDoc} from '../FunctionDescription' export const MATRIX_FUNCTIONS_DOCS: Record = { MAXPOOL: { category: 'Matrix functions', - shortDescription: 'Calculates a smaller range which is a maximum of a window_size, in a given range, for every stride element.', - parameters: [{name: 'range', description: 'The range of numeric values to pool; must contain only numbers.'}, {name: 'window_size', description: 'The width and height, in cells, of the square window whose maximum is taken at each step.'}, {name: 'stride', description: 'The number of cells the window moves between steps; defaults to window_size when omitted.'}], + shortDescription: 'Calculates a smaller range which is a maximum of a window_size, in a given range, for every stride element.
window_size and stride must be positive integers, and the window must tile the range exactly: window_size cannot exceed either dimension of range, and both dimensions, reduced by window_size, must be whole multiples of stride. Otherwise the function returns the #VALUE! error.', + parameters: [{name: 'range', description: 'The range of numeric values to pool; must contain only numbers, and its dimensions must fit a whole number of windows (see window_size and stride).'}, {name: 'window_size', description: 'The width and height, in cells, of the square window whose maximum is taken at each step; a positive integer that is not greater than either dimension of range.'}, {name: 'stride', description: 'The number of cells the window moves between steps; a positive integer that defaults to window_size when omitted. Both dimensions of range, reduced by window_size, must be whole multiples of it, otherwise the function returns the #VALUE! error.'}], documentationUrl: 'https://hyperformula.handsontable.com/docs/guide/built-in-functions.html', examples: ['=MAXPOOL(A1:D4, 2)', '=MAXPOOL(A1:D4, 2, 1)'], }, MEDIANPOOL: { category: 'Matrix functions', - shortDescription: 'Calculates a smaller range which is a median of a window_size, in a given range, for every stride element.', - parameters: [{name: 'range', description: 'The range of numeric values to pool; must contain only numbers.'}, {name: 'window_size', description: 'The width and height, in cells, of the square window whose median is taken at each step.'}, {name: 'stride', description: 'The number of cells the window moves between steps; defaults to window_size when omitted.'}], + shortDescription: 'Calculates a smaller range which is a median of a window_size, in a given range, for every stride element.
window_size and stride must be positive integers, and the window must tile the range exactly: window_size cannot exceed either dimension of range, and both dimensions, reduced by window_size, must be whole multiples of stride. Otherwise the function returns the #VALUE! error.', + parameters: [{name: 'range', description: 'The range of numeric values to pool; must contain only numbers, and its dimensions must fit a whole number of windows (see window_size and stride).'}, {name: 'window_size', description: 'The width and height, in cells, of the square window whose median is taken at each step; a positive integer that is not greater than either dimension of range.'}, {name: 'stride', description: 'The number of cells the window moves between steps; a positive integer that defaults to window_size when omitted. Both dimensions of range, reduced by window_size, must be whole multiples of it, otherwise the function returns the #VALUE! error.'}], documentationUrl: 'https://hyperformula.handsontable.com/docs/guide/built-in-functions.html', examples: ['=MEDIANPOOL(A1:D4, 2)', '=MEDIANPOOL(A1:D4, 2, 1)'], }, diff --git a/src/interpreter/plugin/MatrixPlugin.ts b/src/interpreter/plugin/MatrixPlugin.ts index b2d329d95..8b384c2d4 100644 --- a/src/interpreter/plugin/MatrixPlugin.ts +++ b/src/interpreter/plugin/MatrixPlugin.ts @@ -9,6 +9,7 @@ import {ErrorMessage} from '../../error-message' import {AstNodeType, ProcedureAst} from '../../parser' import {InterpreterState} from '../InterpreterState' import {InternalScalarValue, InterpreterValue} from '../InterpreterValue' +import {Maybe} from '../../Maybe' import {SimpleRangeValue} from '../../SimpleRangeValue' import {FunctionArgumentType, FunctionPlugin, FunctionPluginTypecheck, ImplementedFunctions} from './FunctionPlugin' @@ -37,6 +38,45 @@ function arraySizeForPoolFunction(inputArray: ArraySize, windowSize: number, str ) } +/** + * Checks whether a square pooling window tiles the input array exactly, so that no window reaches outside of it. + * + * The window size and the stride have to be positive integers, the window has to fit inside the input array, + * and both of its dimensions, reduced by the window size, have to be whole multiples of the stride. + * + * @param inputArray - dimensions of the pooled input array + * @param windowSize - side length of the square pooling window + * @param stride - distance between the top-left corners of two consecutive windows + */ +function isPoolWindowFittingInputArray(inputArray: ArraySize, windowSize: number, stride: number): boolean { + return Number.isInteger(windowSize) && windowSize >= 1 + && Number.isInteger(stride) && stride >= 1 + && windowSize <= inputArray.width + && windowSize <= inputArray.height + && (inputArray.width - windowSize) % stride === 0 + && (inputArray.height - windowSize) % stride === 0 +} + +/** + * Validates the arguments shared by the pooling functions (MAXPOOL, MEDIANPOOL). + * + * @param matrix - the pooled input range + * @param windowSize - side length of the square pooling window + * @param stride - distance between the top-left corners of two consecutive windows + * @returns a {@link CellError} describing the violated constraint, or `undefined` when the arguments are valid + */ +function poolFunctionArgumentsError(matrix: SimpleRangeValue, windowSize: number, stride: number): Maybe { + if (!matrix.hasOnlyNumbers()) { + return new CellError(ErrorType.VALUE, ErrorMessage.NumberRange) + } + + if (!isPoolWindowFittingInputArray(matrix.size, windowSize, stride)) { + return new CellError(ErrorType.VALUE, ErrorMessage.PoolDimensions) + } + + return undefined +} + export class MatrixPlugin extends FunctionPlugin implements FunctionPluginTypecheck { public static implementedFunctions: ImplementedFunctions = { 'MMULT': { @@ -61,8 +101,8 @@ export class MatrixPlugin extends FunctionPlugin implements FunctionPluginTypech sizeOfResultArrayMethod: 'maxpoolArraySize', parameters: [ {argumentType: FunctionArgumentType.RANGE}, - {argumentType: FunctionArgumentType.NUMBER}, - {argumentType: FunctionArgumentType.NUMBER, optionalArg: true}, + {argumentType: FunctionArgumentType.INTEGER, minValue: 1}, + {argumentType: FunctionArgumentType.INTEGER, minValue: 1, optionalArg: true}, ], vectorizationForbidden: true, }, @@ -71,8 +111,8 @@ export class MatrixPlugin extends FunctionPlugin implements FunctionPluginTypech sizeOfResultArrayMethod: 'medianpoolArraySize', parameters: [ {argumentType: FunctionArgumentType.RANGE}, - {argumentType: FunctionArgumentType.NUMBER}, - {argumentType: FunctionArgumentType.NUMBER, optionalArg: true}, + {argumentType: FunctionArgumentType.INTEGER, minValue: 1}, + {argumentType: FunctionArgumentType.INTEGER, minValue: 1, optionalArg: true}, ], vectorizationForbidden: true, }, @@ -110,10 +150,19 @@ export class MatrixPlugin extends FunctionPlugin implements FunctionPluginTypech return arraySizeForMultiplication(left, right) } + /** + * Corresponds to MAXPOOL(Range, Window_size, Stride). + * + * Reduces the input range to the maximum value of every window of `Window_size` x `Window_size` cells, + * moving the window by `Stride` cells. The window has to fit inside the range and the range dimensions, + * reduced by the window size, have to be whole multiples of the stride. Otherwise, the function + * returns the #VALUE! error. + */ public maxpool(ast: ProcedureAst, state: InterpreterState): InterpreterValue { return this.runFunction(ast.args, state, this.metadata('MAXPOOL'), (matrix: SimpleRangeValue, windowSize: number, stride: number = windowSize) => { - if (!matrix.hasOnlyNumbers()) { - return new CellError(ErrorType.VALUE, ErrorMessage.NumberRange) + const argumentsError = poolFunctionArgumentsError(matrix, windowSize, stride) + if (argumentsError !== undefined) { + return argumentsError } const outputSize = arraySizeForPoolFunction(matrix.size, windowSize, stride) @@ -133,10 +182,19 @@ export class MatrixPlugin extends FunctionPlugin implements FunctionPluginTypech }) } + /** + * Corresponds to MEDIANPOOL(Range, Window_size, Stride). + * + * Reduces the input range to the median value of every window of `Window_size` x `Window_size` cells, + * moving the window by `Stride` cells. The window has to fit inside the range and the range dimensions, + * reduced by the window size, have to be whole multiples of the stride. Otherwise, the function + * returns the #VALUE! error. + */ public medianpool(ast: ProcedureAst, state: InterpreterState): InterpreterValue { return this.runFunction(ast.args, state, this.metadata('MEDIANPOOL'), (matrix: SimpleRangeValue, windowSize: number, stride: number = windowSize) => { - if (!matrix.hasOnlyNumbers()) { - return new CellError(ErrorType.VALUE, ErrorMessage.NumberRange) + const argumentsError = poolFunctionArgumentsError(matrix, windowSize, stride) + if (argumentsError !== undefined) { + return argumentsError } const outputSize = arraySizeForPoolFunction(matrix.size, windowSize, stride) @@ -227,9 +285,7 @@ export class MatrixPlugin extends FunctionPlugin implements FunctionPluginTypech } } - if (window > array.width || window > array.height - || stride > window - || (array.width - window) % stride !== 0 || (array.height - window) % stride !== 0) { + if (!isPoolWindowFittingInputArray(array, window, stride)) { return ArraySize.error() }