Feature/take function - #1722
Conversation
✅ Deploy Preview for hyperformula-dev-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
hyperformula-docs | a18429c | Commit Preview URL Branch Preview URL |
Aug 13 2026, 10:50 AM |
Performance comparison of head (a18429c) vs base (61ead73) |
| columnsToTake, | ||
| rowsToTake, | ||
| ) | ||
| return SimpleRangeValue.onlyRange(resultRange, this.dependencyGraph) |
There was a problem hiding this comment.
Returning a range-backed value here changes the result's identity, not just how it is read. SimpleRangeValue.onlyRange leaves _data undefined and sets range, so isAdHoc() becomes false — and coerceRangeToScalar (src/interpreter/ArithmeticHelper.ts:813-832) branches on exactly that flag:
if (arg.isAdHoc()) {
return arg.data[0]?.[0]
}
const range = arg.range!
if (state.formulaAddress.sheet === range.sheet) {
if (range.width() === 1) {
const offset = state.formulaAddress.row - range.start.rowSo a TAKE result in scalar context no longer yields its first element; it goes through Excel-style implicit intersection against the source address. Every other array function returns onlyValues and keeps the first-element behaviour.
Verified by running the engine at this commit and at b509e68c7~1, default config (useArrayArithmetic: false), Data!A1:C3 = 1..9:
| formula | this commit | b509e68c7~1 |
|---|---|---|
=TAKE(Data!A1:C3,2)+0 |
#VALUE! "Cell range not allowed." |
1 |
=ABS(TAKE(Data!A1:C3,2)) |
#VALUE! |
1 |
=TAKE(Data!A1:C3,2)&"" |
#VALUE! |
"1" |
The controls in the same sheet are unaffected — =SORT(Data!A1:C3)+0, =VSTACK(Data!A1:C3)+0 and =ARRAY_CONSTRAIN(Data!A1:C3,2,3)+0 all return 1 on both revisions. TAKE now behaves like a bare range reference (=Data!A1:C3+0 → #VALUE!).
Cross-sheet is broken unconditionally, because the state.formulaAddress.sheet === range.sheet guard fails and the function falls through to return undefined:
Sheet1!A1:A2 = =ABS(TAKE(Sheet2!A1:A5,3))
this commit -> #VALUE!, #VALUE!
b509e68c7~1 -> 1, 1
When source and formula share a sheet and the source is 1-D, the intersection succeeds — so there is no error, just the wrong number, and which number depends on where the formula sits. With A1:A4 = 10,20,30,40:
B1:B4 = =ROUND(TAKE($A$1:$A$4,2),0)
this commit -> 10, 20, #VALUE!, #VALUE!
b509e68c7~1 -> 10, 10, 10, 10
One more consequence: the behaviour now depends on the shape of the input rather than on what TAKE does, because a computed source has range === undefined and falls through to the onlyValues path on line 248. =ROUND(TAKE(VSTACK($A$1:$A$4),2),0) returns 10 in every row — wrapping the source in a no-op changes the answer.
This is the caveat from the earlier thread on this line ("Worth confirming the returned-value path behaves") coming due. The LookupPlugin precedent does not carry over: those onlyRange values are consumed inside doVlookup/doHlookup and never escape, so the flag never reaches a coercion site. TAKE returns one as the formula's value.
Nothing in function-take.spec.ts uses TAKE as an argument to anything, so all 35 cases stay green.
If the laziness is worth keeping, the narrowing needs to happen without changing the result's identity — read only the sub-range's cells and return SimpleRangeValue.onlyValues, or let SimpleRangeValue carry narrowed provenance separately from isAdHoc(). A subRange(topRow, leftCol, height, width) method on SimpleRangeValue would keep that decision behind the class boundary and let ARRAY_CONSTRAIN reuse it.
| const height = literalRows === undefined ? sourceSize.height : Math.min(sourceSize.height, literalRows) | ||
| const width = literalColumns === undefined ? sourceSize.width : Math.min(sourceSize.width, literalColumns) | ||
|
|
||
| if (height < 1 || width < 1) { |
There was a problem hiding this comment.
This guard is missing the half that catches non-finite dimensions, so a whole-column or whole-row source silently loses data.
ArraySizePredictor.checkArraySizeForAst returns new ArraySize(range.width(), range.height(), true) for COLUMN_RANGE/ROW_RANGE (src/ArraySize.ts:58-65), and those ranges are constructed with an infinite end — AbsoluteColumnRange sets end.row = Number.POSITIVE_INFINITY (src/AbsoluteCellRange.ts:445), AbsoluteRowRange sets end.col = Number.POSITIVE_INFINITY (:499). When the matching count is not a bare numeric literal, lines 273-274 pass that Infinity straight through, Infinity < 1 is false, and an ArraySize with an infinite axis reaches the vertex.
arrayconstrainArraySize, 134 lines above in this same file, has the clause that catches it:
if (height < 1 || width < 1 || !Number.isInteger(height) || !Number.isInteger(width)) {
return ArraySize.error()
}Verified by running the engine at this commit, Data!A1:C3 = 1..9:
| formula | result | expected |
|---|---|---|
=TAKE(Data!1:3,2) |
[[1],[4]], dims 1x2 |
[[1,2,3],[4,5,6]] |
=TAKE(Data!A:C,,2) |
[[1,2]], dims 2x1 |
[[1,2],[4,5],[7,8]] |
=ARRAY_CONSTRAIN(Data!1:3,2,3) |
[[1,2,3],[4,5,6]] |
correct |
No error is raised — B1, C1, B2, C2 are simply null. The values TAKE computes are right and only the reservation is wrong: in the same sheet =COLUMNS(TAKE(Data!1:3,2)) returns 3 and =SUM(TAKE(Data!1:3,2)) returns 21, while the spilled area is one column wide.
The same formulas placed anywhere other than row 1 / column A fail differently, because ArrayFormulaVertex.getRange() → AbsoluteCellRange.spanFromOrUndef (src/AbsoluteCellRange.ts:117-126) returns undefined for an infinite span not anchored at index 0, and isThereSpaceForArray then reports no space:
Sheet1!B1 = =TAKE(Data!1:3,2) -> #SPILL! "No space for array result."
There is a second, subtler case on the same lines even with a literal count, because the prediction uses the range's declared height while the runtime uses effectiveHeight:
Data = [[1,2,3]] // one row
Sheet1!A1 = =TAKE(Data!A:C,3) // min(Infinity, 3) = 3 predicted, effectiveHeight = 1
The values are correct (1,2,3 in row 1), but A2:C3 are reserved as blanks and isCellPartOfArray(A3) is true; writing anything there turns the formula into #SPILL!. Excel returns a 1x3 result. Clamping the predicted size to the effective dimensions — or reusing ARRAY_CONSTRAIN's Number.isInteger guard so an unbounded source degrades to ArraySize.error() the way it already does for that function — covers both.
No test in function-take.spec.ts uses a whole-column or whole-row source, which is why this is green today.
There was a problem hiding this comment.
Fixed this in takeArraySize(). Bounded cases like TAKE(A:A, 2) still work, while results that remain unbounded return #VALUE!.
I also added regression tests and documented the Excel difference.
| new InterpreterState(state.formulaAddress, state.arraysFlag || (metadata?.enableArrayArithmeticForArguments ?? false)), | ||
| ) | ||
|
|
||
| const literalRows = ArrayPlugin.parseTakeLiteralDimension(ast.args[1]) |
There was a problem hiding this comment.
parseTakeLiteralDimension matches only AstNodeType.NUMBER and a unary +/- wrapping a NUMBER, so every other way of writing a constant count returns undefined and lines 273-274 fall back to the full source size. The reservation is then decided by the spelling of the count rather than by its value.
Verified at this commit, Data 3x3, Sheet1 = [['=TAKE(Data!A1:C3,<count>)'], [null], ['neighbour']] — every one of these results is a single row that fits in A1:C1:
<count> |
result |
|---|---|
1 |
1, 2, 3 — correct |
"1" |
#SPILL! |
TRUE() |
#SPILL! |
(1) |
#SPILL! |
0+1 |
#SPILL! |
2-1 |
#SPILL! |
200% |
#SPILL! |
--1 |
#SPILL! |
Counts!A1 (= 1) |
#SPILL! |
Even with nothing in the way the surplus is reserved: isCellPartOfArray(A3) is true, and a later setCellContents(A3, 'hello') flips the formula to #SPILL!.
Two of these are inputs the spec explicitly supports — 'coerces numeric text to a count' (function-take.spec.ts:293) and 'coerces TRUE to one' (:300) — and both pass only because they run on an otherwise empty sheet. Note ArraySizePredictor.checkArraySizeForAst unwraps PARENTHESIS (src/ArraySize.ts:115) and runFunctionWithReferenceArgument unwraps it in a while loop, so skipping it here is not a house convention.
The cell-reference case is genuinely unfixable at parse time and an upper bound is the only option there — same situation FILTER and UNIQUE are in. But the constant forms above are all statically known, and the helper this one was modelled on already handles most of them. SequencePlugin.parseLiteralDimension (src/interpreter/plugin/SequencePlugin.ts:41-61) covers NUMBER, STRING, both unary ops, and zero-arg TRUE()/FALSE():
if (node.type === AstNodeType.STRING) {
const parsed = Number(node.value)
return Number.isFinite(parsed) ? Math.trunc(parsed) : undefined
}So =SEQUENCE("2") predicts its size and =TAKE(A1:C3,"2") does not — an inconsistency between two functions in the same category with no reason behind it.
This is now the fourth hand-rolled copy of "extract a static number from an AST node": here, SequencePlugin.parseLiteralDimension, arrayconstrainArraySize (line 136 of this file, NUMBER only), and FormulaParser.handleOffsetHeuristic (src/parser/FormulaParser.ts:766-790, inlined four times). They disagree on capability, and the next one lands with DROP/CHOOSEROWS/EXPAND. Extracting a single signedNumberLiteralValue(ast): number | undefined into src/parser/Ast.ts — which already owns AstNodeType and the unary-op builders — and calling it from all four would fix this class in one place; the Math.abs stays at TAKE's call site since only TAKE wants the magnitude.
Worth pinning whatever the final boundary is with tests, since none of the rows in the table above is covered today.
| */ | ||
| export enum ErrorType { | ||
| /** Calculation error. */ | ||
| CALC = 'CALC', |
There was a problem hiding this comment.
We need to take CALC out of this PR. Adding a member to ErrorType is a breaking change to a public API, and we don't want to cut a major release now.
Why it's breaking
TranslationPackage's constructor validates error and UI keys exhaustively against the enum:
// src/i18n/TranslationPackage.ts
private checkErrors(): void {
for (const key of Object.values(ErrorType)) {
if (!(key in this.errors) && (key !== ErrorType.LIC)) {
throw new MissingTranslationError(`errors.${key}`)
}
}
}So any third-party language pack written against 3.3.0 now fails at registration, before a single formula is evaluated:
HyperFormula.registerLanguage('myPack', packBuiltAgainst_3_3_0)
// MissingTranslationError: Translation for errors.CALC is missing in the translation package you're using.
RawTranslationPackage is public API and custom packs are a documented feature — docs/guide/localizing-functions.md even walks users through building one, and the two example packs on that page list the error keys explicitly, so they throw as of this branch.
Worth being precise about the asymmetry, because it is not obvious: adding a function is not breaking in the same way. checkFunctionTranslations doesn't iterate the registry at all — it only rejects packs that override protected names — so a pack that has never heard of TAKE registers fine and =TAKE(...) simply resolves to #NAME? in that language (FunctionRegistry.getFunction gates on isFunctionTranslated). Verified against this branch, each pack derived from enGB with one key removed:
| pack | registerLanguage |
|---|---|
missing functions.TAKE |
OK — =TAKE(...) → #NAME? |
missing functions.SORT |
OK — =SORT(...) → #NAME? |
functions: {} (empty) |
OK |
missing errors.CALC |
throws |
missing ui.NEW_SHEET_PREFIX |
throws |
So the TAKE half of this PR is genuinely additive; the CALC half breaks packs on its own, even for users who never call the function. For precedent: #SPILL!, the last translatable error type we added, shipped in 1.0.0.
To be clear — #CALC! is the right answer
Microsoft is explicit: "Excel returns a #CALC! error to indicate an empty array when either rows or columns is 0." This isn't a case where we should pick something else on the merits. It's purely a release-timing constraint.
Interim solution
Use ErrorType.NA and keep the new message:
return new CellError(ErrorType.NA, ErrorMessage.ZeroRowOrColumnCount)#N/A is the closest fit among the existing types because it is already what this codebase returns for an empty array result — FILTER does exactly that 37 lines above in this same file:
return new CellError(ErrorType.NA, ErrorMessage.EmptyRange)and UNIQUE and SORT agree. Using #N/A keeps TAKE consistent with its own family instead of introducing a third convention (SEQUENCE(0) returns #VALUE!, which is the other candidate but the odd one out). Keeping ErrorMessage.ZeroRowOrColumnCount preserves the accuracy fix from the earlier round — the message stays correct about what was zero, only the error type changes.
That leaves us with one migration later rather than two, since we'll likely want to move FILTER/UNIQUE/SORT/SEQUENCE to #CALC! at the same time.
What to revert here
CALC = 'CALC'insrc/Cell.ts- the
CALCentry in all 17 language packs (this also makes the "every pack ships the untranslated English#CALC!" problem moot for now — when we do add it, the packs need real localized names; Polish Excel uses#OBL!) - the
#CALC!row indocs/guide/types-of-errors.md - the
CALChalf of the CHANGELOG entry
Keep ErrorMessage.ZeroRowOrColumnCount, and record the deviation in docs/guide/list-of-differences.md — there's already a row of exactly this shape for =SEQUENCE(0) (VALUE / N/A / CALC).
Tracked for the next major in HF-350 (tagged breaking change), which has the full scope and the reasoning above. We'll do it there.
There was a problem hiding this comment.
Fixed in c8924ae. Removed ErrorType.CALC, its entries from all 17 language packs, and the related error documentation and changelog text. TAKE now returns ErrorType.NA with ErrorMessage.ZeroRowOrColumnCount for zero counts.
Added regression coverage in a9f3334 confirming that language packs without a CALC translation register successfully, and documented the Excel deviation.
| * | ||
| * @param {FunctionArgument | undefined} argument - The argument metadata to inspect. | ||
| */ | ||
| export function isFunctionArgumentOptional(argument: FunctionArgument | undefined): boolean { |
There was a problem hiding this comment.
Same situation as CALC: the problem is real and the approach is sound, but it's breaking and we can't afford a major right now — so let's solve it locally inside TAKE instead of changing the shared model.
The problem you hit is real
TAKE genuinely needs something the argument-metadata model cannot express: rows must accept a syntactically empty slot (=TAKE(range,,2) → keep all rows) while still rejecting an omitted argument (=TAKE(range) → error, since Excel marks rows Required). emptyAsDefault is inert without a defaultValue, and a defaultValue used to imply optional — so "required, but empty allowed" had no expression.
It's also worth saying why nothing else in the codebase hit this: TAKE is the only function where an emptyAsDefault argument is positionally required. Every other user of the flag puts it on a genuinely optional trailing argument, where "omitted" and "empty" should mean the same thing:
| function | parameters with emptyAsDefault |
shape |
|---|---|---|
ADDRESS |
abs_number, use_a1 |
trailing, optionalArg: true |
SEQUENCE |
columns, start, step |
trailing, optional |
SORT |
sort_index, sort_order, by_col |
trailing, optional |
UNIQUE |
by_col, exactly_once |
trailing, optional |
TAKE |
rows |
middle, required |
So the gap is genuine, and separating defaults from optionality is the right long-term model.
Why we can't ship it now
isFunctionArgumentOptional changes the meaning of an existing combination on a public interface. The old predicate was optionalArg || defaultValue !== undefined; the new one returns optionalArg verbatim when it is set, so {optionalArg: false, defaultValue: X} flips from optional to required.
FunctionArgument is exported from src/index.ts and custom function plugins are a documented feature. Verified by registering a plugin whose 2nd parameter is {argumentType: NUMBER, optionalArg: false, defaultValue: 42}:
=OPTFN(1) -> 43 on ebaa2b28a
=OPTFN(1) -> #N/A "Wrong number of arguments." on this branch
getFunctionDetails('OPTFN').parameters[1].optional also flips true → false, so metadata-driven autocomplete starts advertising the wrong arity.
Two things make it worse than a typical semantic tweak. First, no built-in uses that combination except TAKE itself, so the entire blast radius is user code and nothing in-tree would ever catch a regression. Second, docs/guide/custom-functions.md actively prescribes the combination — its MY_FUNCTION example pairs defaultValue: 10 with optionalArg: false, and the page still states the old rules ("Setting a defaultValue for an argument always makes that argument optional"). A plugin copy-pasted from our own published guide changes arity on upgrade.
Interim solution: check the arity inside TAKE
Revert isFunctionArgumentOptional to the previous predicate, drop optionalArg: false from the rows parameter, and enforce the requirement where it belongs — in the function that has the requirement:
public take(ast: ProcedureAst, state: InterpreterState): InterpreterValue {
// `rows` carries a default so that a syntactically empty slot keeps every row, which also makes
// the argument metadata treat it as omittable. TAKE requires it, so the arity is checked here.
if (ast.args.length < 2) {
return new CellError(ErrorType.NA, ErrorMessage.WrongArgNumber)
}
return this.runFunction(ast.args, state, this.metadata('TAKE'), ...)
}runFunction still rejects the 4-argument call on its own, and takeArraySize already guards ast.args.length < 2 || > 3, so this is the only gap to close.
I built it and ran it. Every user-visible TAKE behaviour is identical to this branch:
| formula | this branch | with the local check |
|---|---|---|
=TAKE(A1:C3) |
#N/A |
#N/A |
=TAKE(A1:C3,,2) |
[[1,2],[4,5],[7,8]] |
same |
=TAKE(A1:C3,2) |
[[1,2,3],[4,5,6]] |
same |
=TAKE(A1:C3,) / =TAKE(A1:C3,,) |
full source | same |
=TAKE(A1:C3,2,) |
[[1,2,3],[4,5,6]] |
same |
=TAKE(A1:C3,-2,-2) |
[[5,6],[8,9]] |
same |
=TAKE(A1:C3,0,2) |
zero-count error | same |
=TAKE(A1:C3,1,1,1) |
#N/A |
same |
=OPTFN(1) (third-party plugin above) |
#N/A |
43 — restored |
All 35 cases in function-take.spec.ts pass unchanged.
The cost, stated honestly
getFunctionDetails('TAKE').parameters[1].optional becomes true instead of false. The public metadata will advertise rows as optional even though the function rejects the one-argument call — Excel documents it as Required. That's a real (if cosmetic) regression, and it's the price of not touching the shared predicate. The catalogue can't override it, since optionality is derived entirely from implementedFunctions.
Concretely that means three assertions in the tests repo need updating — the full run was 6176 passed / 3 failed, and all three are assertions that encode the new shared-model semantics rather than TAKE's behaviour:
optional-parameters.spec.ts→ "uses a required argument default only for syntactically empty input" (theREQUIREDDEFAULTTESTplugin)function-metadata-api.spec.ts→ "returns full details for TAKE" (theoptionalflags)function-metadata-api.spec.ts→ "derives optionality from the implementation on the fallback path"
Reverting also means docs/guide/custom-functions.md needs no change — it becomes correct again as written.
For the record: the right fix, later
The underlying issue is that defaultValue does two jobs — "value when the argument is omitted" and "value when the slot is empty". Splitting them (a separate emptyValue, or letting emptyAsDefault carry its own value) lets TAKE declare no defaultValue at all: it stays required under the existing optionality rule, empty slots still work, no existing flag changes meaning, and nothing third-party breaks. I prototyped that too and it gives optional: [false, false, true] with the full suite green apart from the same three assertions. Worth a backlog item alongside the #CALC! one — happy to file it if you want.
There was a problem hiding this comment.
Restored the established behavior where arguments with a defaultValue are treated as optional, and removed optionalArg: false from TAKE’s rows metadata
The required argument-position rule is now localized to TAKE: TAKE(array) returns #N/A with WrongArgNumber, while TAKE(array, , columns) remains valid and uses the default row count.
Added coverage for default-value optionality, public function metadata, and TAKE’s argument-count behavior.
Implementation: 6804d04
Tests: 9685c61
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b14d8ec. Configure here.
TAKE shipped with the English name in 10 of 16 packs while 6 carried a
translation, and VSTACK/HSTACK were English in all 16. Microsoft localizes
all three in most locales, so a user could not type the name their Excel
uses.
Names taken from Microsoft's localized "Excel functions (alphabetical)"
page, one locale at a time. Each row there links to the function's own page
using the English slug in the href while the link text is the localized
name, so the lookup is exact:
<a href="functions/take-function">WYCINEK</a>
Left as English where Microsoft itself does not translate: TAKE, VSTACK and
HSTACK in Indonesian, and VSTACK/HSTACK in Swedish.
Note that a function's own localized page is not a usable source: for
several locales its syntax block still shows the English name even though
the prose and argument names are translated (the French page shows
"=TAKE(tableau, lignes,[colonnes])" while the product uses PRENDRE).
DEV_DOCS records the lookup method and adds the governing policy: ship a
localized name only when it can be confirmed against the product, and keep
the English name otherwise, since an invented name matches nothing, reads
plausibly enough to be typed first, and fails as #NAME?.
No changelog entry: TAKE, VSTACK and HSTACK are all still in [Unreleased],
so no wrong name has been released.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1118288 to
a18429c
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1722 +/- ##
===========================================
+ Coverage 97.31% 97.33% +0.02%
===========================================
Files 195 195
Lines 15719 15778 +59
Branches 3455 3472 +17
===========================================
+ Hits 15297 15358 +61
+ Misses 414 412 -2
Partials 8 8
🚀 New features to boost your workflow:
|
2 participants
Context
This PR adds support for the
TAKEdynamic-array function.TAKEreturns a specified number of rows or columns from the beginning or end of an array. The implementation supports positive and negative counts, optional columns, syntactically empty argument slots, array spilling, and function metadata.It also introduces the
CALCerror type and its#CALC!representation in all built-in language packs. For now,#CALC!is produced only byTAKEwhen a row or column count evaluates to zero, including fractional values truncated to zero and blank-cell references coerced to zero.Omitting the required
rowsargument still returns the existing wrong-argument#N/Aerror.How did you test your changes?
TAKE, covering:#CALC!.#N/A.TAKEtest suite successfully: 20 tests passed.npm run verify:typingssuccessfully.npm run lintsuccessfully with zero errors.Types of changes
Related issues:
Checklist:
Note
Medium Risk
Changes dynamic-array spill sizing and shared optional-argument validation in
FunctionPlugin, which affects how many functions accept omitted args; TAKE itself is new surface area with Excel parity nuances (zero counts, unbounded columns).Overview
Adds the TAKE dynamic-array function so formulas can return a spilled sub-range from the start or end of an array, with optional row/column counts, syntactically empty argument slots, and spill-size prediction that rejects unbounded whole-column results.
Implementation lives in
ArrayPlugin(take/takeArraySize): positive/negative counts, defaults viaPOSITIVE_INFINITYandemptyAsDefault, lazy slicing for address-backed ranges, and#N/Awhen row or column count is zero (newZeroRowOrColumnCountmessage). Missing the requiredrowsargument still yields wrong-argument#N/A.Shared plumbing:
isFunctionArgumentOptionalcentralizes optional-argument rules; function metadata and lookup docs register TAKE; VSTACK/HSTACK get locale-specific names in several packs; changelog and Excel comparison table document TAKE behavior vs Excel (e.g. whole-columnTAKE).Reviewed by Cursor Bugbot for commit a18429c. Bugbot is set up for automated code reviews on this repo. Configure here.