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 | 8d9e74e | Commit Preview URL Branch Preview URL |
Sep 03 2026, 09:27 PM |
Performance comparison of head (8d9e74e) vs base (c920375) |
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>
|
The description still describes the Two small leftovers from that same removal:
|
|
@marcin-kordas-hoc Thanks for flagging this. I moved the VSTACK/HSTACK localization fix into its own PR so the breaking language-pack migration can be reviewed separately: #1748 (companion tests: 44). The changes are no longer part of this TAKE PR. |
|
A nested TAKE returns a spurious
|
A non-foldable count over an unbounded source returns
|
| formula | in A1 |
in A2 |
vertex at A2 |
|---|---|---|---|
=TAKE(Data!A:A,Counts!A1) |
spills 1, 4 |
#VALUE! "Cell range not allowed." |
scalar (isCellPartOfArray false) |
=FILTER(Data!A:A,Data!A:A>0) |
spills | #SPILL! "No space for array result." |
array |
=SORT(Data!A:A) |
spills | #SPILL! |
array |
=UNIQUE(Data!A:A) |
spills | #SPILL! |
array |
=VSTACK(Data!A:A) |
spills | #SPILL! |
array |
=TAKE(Data!A:A,,1) |
spills | #SPILL! |
— |
=TAKE(Data!A:A,2) |
spills 1, 4 |
spills 1, 4 |
array |
So TAKE is inconsistent with FILTER, with three other array siblings, and with its own literal-count branch. #VALUE! also isn't a considered choice about TAKE's semantics — it is an internal scalar-expected error leaking out, which is why it has two knock-on symptoms #SPILL! doesn't: it poisons downstream references (=SUM(A2:A3) → #VALUE!) and it never recovers on recalculation, even after the count cell changes.
Root cause: the predictor cannot tell "unbounded" from "unknown"
takeArraySize classifies the count with parseTakeLiteralDimension, which has no AstNodeType.EMPTY branch — an empty argument and an unfoldable one both fall through to {kind: 'unresolved'}, and both then take height = sourceSize.height (Infinity) and hit the startsBelowFirstRow guard. Instrumenting the predictor shows the two producing a byte-identical result:
=TAKE(Data!A:A,,1) args[1..2] astType=["EMPTY","NUMBER"] -> ArraySize {w:1,h:1,isRef:true} scalar=true
=TAKE(Data!A:A,Counts!A1) args[1..2] astType=["CELL_REFERENCE","MISSING"] -> ArraySize {w:1,h:1,isRef:true} scalar=true
=TAKE(Data!A:A,2) args[1..2] astType=["NUMBER","MISSING"] -> ArraySize {w:1,h:2,isRef:false} scalar=false
But the two cases are not the same thing:
- empty argument → the count really is
Infinity(keep all rows), the result really is unbounded, and#SPILL!below row 1 is correct and is what the spec pins. - non-foldable argument → the count is finite, just not known until evaluation. The result is 2 rows and fits perfectly at
A2.
The reason they diverge downstream is that ArraySize.error() makes the engine build a scalar vertex in both cases. For the empty argument take()'s own runtime guard (!Number.isFinite(requestedRows), ArrayPlugin.ts:357-358) also fires and returns a scalar #SPILL! CellError, which sits in a scalar vertex happily. For the non-foldable argument the evaluated count is finite, the guard correctly does not fire, and take() returns a SimpleRangeValue into a scalar vertex — which Exporter can only render as #VALUE!.
Note that the runtime half is already right: it keys off the evaluated count, so it treats these two cases differently, exactly as it should. Only the predictor conflates them.
Suggested fix — adopt FILTER's model for the "unknown" case
filterArraySize never collapses an unresolvable size into an error; it takes the source's size as an upper bound and lets the engine's generic spill machinery decide whether it fits:
// filterArraySize — ArrayPlugin.ts:313-314
const width = Math.max(...(subChecks).map(val => val.width))
const height = Math.max(...(subChecks).map(val => val.height))
return new ArraySize(width, height)Concretely, in takeArraySize:
- Give the count classification a third outcome, distinguishing an
AstNodeType.EMPTYargument (genuinely unbounded) from an unfoldable non-empty one (finite, unknown) — today both are'unresolved'. Akind: 'unbounded'alongside'unresolved'would do it. - Restrict
startsBelowFirstRow/startsRightOfFirstColumn(ArrayPlugin.ts:415-416) to the genuinely-unbounded case, so the existing#SPILL!behaviour for=TAKE(Data!A:A,,1)and its three sibling spec tests is unchanged. - For the unknown-but-finite case, return
new ArraySize(effectiveWidth, effectiveHeight)— the source's effective footprint as an upper bound, which the method already computes and already returns for this exact formula when it sits in row 1. That is theFILTERmodel: an honest array vertex, so the engine yields a real spill where it fits and an honest#SPILL!where it does not.
That keeps take() untouched, leaves every one of the 69 existing TAKE tests passing, and lines TAKE up with FILTER/SORT/UNIQUE/VSTACK and with its own literal-count branch in one change.
Worth adding tests for a cell-reference, cross-sheet-reference and function-call count over a whole-column and a whole-row source, at row 1 and below — the spec currently has 12 unbounded-source tests and every one uses a literal or omitted count, which is why this path is unguarded.
Two caveats so this isn't over-read. |count| == 1 accidentally works today (Interpreter unwraps a 1×1 SimpleRangeValue), so the bug only bites for |count| >= 2. And separately, FILTER at row 1 spills only its first row rather than the full used height — a pre-existing quirk in how these siblings realise unbounded footprints, not something this PR should take on.
Generated by Claude Code
|
|
Fixed in The runtime position guard has been removed. For a direct unbounded Regression coverage now verifies both a nested whole-column result below row 1 and a nested whole-row result after column A. The existing four direct top-level |
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 5b1bd12. Configure here.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1722 +/- ##
===========================================
+ Coverage 97.32% 97.36% +0.04%
===========================================
Files 195 195
Lines 15739 15875 +136
Branches 3390 3526 +136
===========================================
+ Hits 15318 15457 +139
+ Misses 421 410 -11
- Partials 0 8 +8
🚀 New features to boost your workflow:
|
|
Thanks for flagging this. The HyperFormula behavior issue was valid, though the suggestion that Google Sheets supports I updated the comparison to reflect the verified behavior:
I verified the Excel behavior directly in Excel and pushed the documentation correction in commit |

Context
This PR adds the
TAKEdynamic-array function.TAKEreturns rows or columns from the beginning or end of an array and supports positive and negative counts, optional columns, syntactically empty argument slots, and array spilling.A row or column count that truncates to zero returns the existing
#N/Aerror with a TAKE-specific message. This is a documented Excel difference because HyperFormula does not expose a#CALC!error type. Omitting the requiredrowsargument continues to return the existing wrong-argument#N/Aerror.Implementation
#SPILL!below row 1; finite counts are capped to configured sheet limits before allocation.Validation
TAKEtests passed.git diff --checkpassed.Types of changes
Checklist
CHANGELOG.md.Note
Medium Risk
New dynamic-array spill logic and size prediction touch array evaluation and sheet limits; behavior is well-tested but mistakes could cause incorrect spills or performance issues on large column references.
Overview
Adds the Excel-style
TAKEdynamic-array function so formulas can return a sub-rectangle of a range or array from the start or end, with optional row/column counts, empty slots meaning “keep all,” and negative counts selecting from the tail.Implementation lives in
ArrayPlugin: counts are truncated and capped to the source size; zero row/column counts yield#N/Awith a newZeroRowOrColumnCountmessage (documented as differing from Excel’s#CALC!). Address-backed sources stay lazy viaAbsoluteCellRangesub-spans.takeArraySizestatically resolves literal count expressions (including simple arithmetic andTRUE/FALSE) for spill sizing; whole-column/row unbounded cases respect sheet limits and row‑1/column‑1 spill boundaries.Interpreter.isFunctionImplementedBysupports that static analysis.Also updates changelog, built-in function metadata, compatibility notes for
TAKE, and localized names across language packs.Reviewed by Cursor Bugbot for commit 8d9e74e. Bugbot is set up for automated code reviews on this repo. Configure here.