-
Notifications
You must be signed in to change notification settings - Fork 154
Add SEQUENCE built-in function #1645
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
Changes from all commits
add3c53
a4ce9a3
de1f362
4b43c6c
37cce52
a138c34
2ca7f0b
29b7f29
e809c59
ce2a218
4fb332c
1bfb26b
f7350a1
1c8d402
d4c0e0c
9e9a05f
57307b2
826ec41
3167a0b
a82a120
732906e
b08cd79
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,176 @@ | ||
| /** | ||
| * @license | ||
| * Copyright (c) 2025 Handsoncode. All rights reserved. | ||
| */ | ||
|
|
||
| import { ArraySize } from '../../ArraySize' | ||
| import { CellError, ErrorType } from '../../Cell' | ||
| import { ErrorMessage } from '../../error-message' | ||
| import { Ast, AstNodeType, ProcedureAst } from '../../parser' | ||
| import { InterpreterState } from '../InterpreterState' | ||
| import { InterpreterValue } from '../InterpreterValue' | ||
| import { SimpleRangeValue } from '../../SimpleRangeValue' | ||
| import { FunctionArgumentType, FunctionPlugin, FunctionPluginTypecheck, ImplementedFunctions } from './FunctionPlugin' | ||
|
|
||
| /** | ||
| * Plugin implementing the SEQUENCE spreadsheet function. | ||
| * | ||
| * SEQUENCE(rows, [cols], [start], [step]) returns a rows×cols array of | ||
| * sequential numbers starting at `start` and incrementing by `step`. | ||
| */ | ||
| export class SequencePlugin extends FunctionPlugin implements FunctionPluginTypecheck<SequencePlugin> { | ||
| /** | ||
| * Minimum valid value for the `rows` and `cols` arguments. | ||
| * Extracted to avoid duplicating the check between `sequence()` (runtime) and | ||
| * `sequenceArraySize()` (parse time). | ||
| */ | ||
| private static readonly MIN_DIMENSION = 1 | ||
|
|
||
| /** Returns true when `n` is a finite number at least {@link MIN_DIMENSION}. */ | ||
| private static isValidDimension(n: number): boolean { | ||
| return Number.isFinite(n) && n >= SequencePlugin.MIN_DIMENSION | ||
| } | ||
|
|
||
| /** | ||
| * Parses a literal dimension from an AST node at parse time. | ||
| * Handles NUMBER nodes directly, STRING nodes via numeric coercion, | ||
| * PLUS/MINUS_UNARY_OP wrapping a NUMBER (e.g. `+3`, `-2`), | ||
| * and TRUE()/FALSE() function calls (returning 1/0). | ||
| * Returns undefined for non-literal nodes (cell refs, formulas, binary ops). | ||
| */ | ||
| private static parseLiteralDimension(node: Ast): number | undefined { | ||
| if (node.type === AstNodeType.NUMBER) { | ||
| return Math.trunc(node.value) | ||
| } | ||
| if (node.type === AstNodeType.STRING) { | ||
| const parsed = Number(node.value) | ||
| return Number.isFinite(parsed) ? Math.trunc(parsed) : undefined | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Locale numeric strings mis-sized at parse timeMedium Severity
Additional Locations (1)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Accepted limitation. Parse-time uses JavaScript Number() which doesn't respect HF locale settings. Documented in list-of-differences. |
||
| } | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (node.type === AstNodeType.PLUS_UNARY_OP && node.value.type === AstNodeType.NUMBER) { | ||
| return Math.trunc(node.value.value) | ||
| } | ||
| if (node.type === AstNodeType.MINUS_UNARY_OP && node.value.type === AstNodeType.NUMBER) { | ||
| return Math.trunc(-node.value.value) | ||
| } | ||
| if (node.type === AstNodeType.FUNCTION_CALL) { | ||
| if (node.procedureName === 'TRUE' && node.args.length === 0) { | ||
| return 1 | ||
| } | ||
| if (node.procedureName === 'FALSE' && node.args.length === 0) { | ||
| return 0 | ||
| } | ||
| } | ||
| return undefined | ||
| } | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Parenthesized dimensions rejected as non-literalMedium Severity
Additional Locations (1)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Accepted limitation. Parenthesized expressions like =SEQUENCE((3)) are not unwrapped at parse time. Minor edge case — users can write =SEQUENCE(3). |
||
|
|
||
| public static implementedFunctions: ImplementedFunctions = { | ||
| 'SEQUENCE': { | ||
| method: 'sequence', | ||
| sizeOfResultArrayMethod: 'sequenceArraySize', | ||
| parameters: [ | ||
| { argumentType: FunctionArgumentType.NUMBER }, | ||
| { argumentType: FunctionArgumentType.NUMBER, defaultValue: 1, emptyAsDefault: true }, | ||
| { argumentType: FunctionArgumentType.NUMBER, defaultValue: 1, emptyAsDefault: true }, | ||
| { argumentType: FunctionArgumentType.NUMBER, defaultValue: 1, emptyAsDefault: true }, | ||
| ], | ||
| vectorizationForbidden: true, | ||
| }, | ||
| } | ||
|
|
||
| /** | ||
| * Corresponds to SEQUENCE(rows, [cols], [start], [step]) | ||
| * | ||
| * Returns a rows×cols array of sequential numbers starting at `start` | ||
| * and incrementing by `step`, filled row-major. | ||
| * | ||
| * Note: dynamic arguments (cell references, formulas) for `rows` or `cols` | ||
| * cause a size mismatch between parse-time prediction and runtime result, | ||
| * which results in a #VALUE! error. Use literal numbers for rows and cols. | ||
| * | ||
| * @param {ProcedureAst} ast - The parsed function call AST node. | ||
| * @param {InterpreterState} state - Current interpreter evaluation state. | ||
| */ | ||
| public sequence(ast: ProcedureAst, state: InterpreterState): InterpreterValue { | ||
| return this.runFunction(ast.args, state, this.metadata('SEQUENCE'), | ||
| (rows: number, cols: number, start: number, step: number) => { | ||
| if (!Number.isFinite(rows) || !Number.isFinite(cols)) { | ||
| return new CellError(ErrorType.VALUE, ErrorMessage.ValueLarge) | ||
| } | ||
|
|
||
| if (rows < 0 || cols < 0) { | ||
| return new CellError(ErrorType.VALUE, ErrorMessage.LessThanOne) | ||
| } | ||
|
|
||
| const numRows = Math.trunc(rows) | ||
| const numCols = Math.trunc(cols) | ||
|
|
||
| if (!SequencePlugin.isValidDimension(numRows) || !SequencePlugin.isValidDimension(numCols)) { | ||
| return new CellError(ErrorType.VALUE, ErrorMessage.LessThanOne) | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Zero dimensions return wrong error typeMedium Severity
Additional Locations (1)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in commit f7350a1 — zero dimensions now return #VALUE! instead of #NUM!, matching Excel. Agreed with reviewer on the call. |
||
|
|
||
| if (numRows > this.config.maxRows || numCols > this.config.maxColumns) { | ||
| return new CellError(ErrorType.VALUE, ErrorMessage.ValueLarge) | ||
| } | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| const result: number[][] = [] | ||
| for (let r = 0; r < numRows; r++) { | ||
| const row: number[] = [] | ||
| for (let c = 0; c < numCols; c++) { | ||
| row.push(start + (r * numCols + c) * step) | ||
| } | ||
| result.push(row) | ||
| } | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| return SimpleRangeValue.onlyNumbers(result) | ||
| } | ||
| ) | ||
| } | ||
|
|
||
| /** | ||
| * Predicts the output array size for SEQUENCE at parse time. | ||
| * | ||
| * Handles NUMBER and STRING literals for rows/cols via `parseLiteralDimension`. | ||
| * Non-literal args (cell refs, formulas, unary/binary ops) fall back to 1, | ||
| * which will cause a size mismatch at eval time when the actual result is larger. | ||
| * | ||
| * @param {ProcedureAst} ast - The parsed function call AST node. | ||
| * @param {InterpreterState} _state - Current interpreter evaluation state (unused). | ||
| */ | ||
| public sequenceArraySize(ast: ProcedureAst, _state: InterpreterState): ArraySize { | ||
| if (ast.args.length < 1 || ast.args.length > 4) { | ||
| return ArraySize.error() | ||
| } | ||
|
|
||
| const rowsArg = ast.args[0] | ||
| const colsArg = ast.args.length > 1 ? ast.args[1] : undefined | ||
|
|
||
| // Non-literal rows (cell ref, formula, unary/binary op): size unknown at parse time. | ||
| // Fall back to scalar so the engine creates a ScalarFormulaVertex instead of an | ||
| // ArrayFormulaVertex. The actual evaluation will propagate errors or return #VALUE! | ||
| // via the Exporter if the result is larger than 1×1. | ||
| if (rowsArg.type === AstNodeType.EMPTY) { | ||
| return ArraySize.error() | ||
| } | ||
| const rows = SequencePlugin.parseLiteralDimension(rowsArg) | ||
| if (rows === undefined) { | ||
| return ArraySize.error() | ||
| } | ||
|
|
||
| const cols = (colsArg === undefined || colsArg.type === AstNodeType.EMPTY) | ||
| ? 1 | ||
| : SequencePlugin.parseLiteralDimension(colsArg) | ||
| if (cols === undefined) { | ||
| return ArraySize.error() | ||
| } | ||
|
|
||
| if (!SequencePlugin.isValidDimension(rows) || !SequencePlugin.isValidDimension(cols)) { | ||
| return ArraySize.error() | ||
| } | ||
|
|
||
| if (rows > this.config.maxRows || cols > this.config.maxColumns) { | ||
| return ArraySize.error() | ||
| } | ||
|
|
||
| return new ArraySize(cols, rows) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,7 +35,7 @@ git fetch origin | |
| if git show-ref --verify --quiet "refs/heads/$CURRENT_BRANCH" || \ | ||
| git show-ref --verify --quiet "refs/remotes/origin/$CURRENT_BRANCH"; then | ||
| git checkout "$CURRENT_BRANCH" | ||
| git pull # pull latest changes | ||
| git pull origin "$CURRENT_BRANCH" # pull latest changes | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pull command fails for local-only branchesLow Severity After checkout,
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed — fetch-tests.sh now uses explicit branch in git pull. |
||
| else | ||
| echo "Branch $CURRENT_BRANCH not found in hyperformula-tests, creating from develop..." | ||
| git checkout develop | ||
|
|
||


Uh oh!
There was an error while loading. Please reload this page.