Conversation
…#498) When Frame.indexRowsWith receives more keys than the frame has rows, the extra rows are now properly represented as missing values. Previously the function produced a frame with an inconsistent state: the row index had more entries than the data vectors, causing Frame.fillMissingWith to silently no-op (for non-float types) or throw an InvalidOperationException (for float). Root cause: VectorConstruction.GetRange always spanned the original row range, leaving the new index longer than the resulting vector. Fix: when newCount > existingCount, append a VectorConstruction.Empty block (all-missing) for the extra rows so the vector length matches the new index length. Co-authored-by: Copilot <[email protected]>
This was referenced Mar 9, 2026
…tra-keys-15abd25a49fa86a1
…tra-keys-15abd25a49fa86a1
…tra-keys-15abd25a49fa86a1
dsyme
deleted the
repo-assist/fix-issue-498-indexrowswith-extra-keys-15abd25a49fa86a1
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 This PR was created by Repo Assist, an automated AI assistant.
Summary
Fixes
Frame.indexRowsWithto correctly handle the case where the caller provides more row keys than the frame currently has rows. Extra keys now produce rows with missing values, consistent with user expectations.Closes #498
Root Cause
indexRowsWithbuilt aVectorConstruction.GetRangecommand that always spanned the original row range. When the caller supplied a longer key sequence the newRowIndexhad more entries than the resulting data vectors (length = original row count), producing an internally inconsistent frame. Downstream operations likeFrame.fillMissingWitheither silently no-opped or threw:Fix
When
newCount > existingCount, the vector command is changed to:This appends an all-missing segment so the vector length matches the new row index. When
newCount <= existingCountthe original behaviour is preserved.An empty-frame edge-case (
existingCount = 0) is handled withVectorConstruction.Empty newCount.Trade-offs
indexRowsWithaccepts the same arguments as before; only the output for longer key sequences changes from "broken frame" to "frame with missing rows".indexColsWithalready validates that key count must equal column count;indexRowsWithnow has richer semantics (pad with missing) instead of strict validation, matching the reasonable user expectation demonstrated inFrame.fillMissingdoes not work withFrame.indexRowsWith#498.Test Status
A regression test was added in
tests/Deedle.Tests/Frame.fsthat:RowCountis the new key countfillMissingWith 0.0fills the extra rows correctlyBuild: ✅⚠️ Not runnable locally — the test project targets
dotnet build src/Deedle/Deedle.fsproj -c Release— succeeded, 0 errorsTest compile: ✅
dotnet build tests/Deedle.Tests/Deedle.Tests.fsproj -c Release— succeeded, 0 errorsTest run:
net5.0but only .NET 8+ is available in this environment. CI (once PR #590 upgrading to .NET 8 lands) will run the full test suite.