Skip to content

Loop instead of recursing in Dataset.diff - #11704

Open
headtr1ck wants to merge 2 commits into
pydata:mainfrom
headtr1ck:fix-diff-recursion
Open

Loop instead of recursing in Dataset.diff#11704
headtr1ck wants to merge 2 commits into
pydata:mainfrom
headtr1ck:fix-diff-recursion

Conversation

@headtr1ck

@headtr1ck headtr1ck commented Oct 9, 2026

Copy link
Copy Markdown
Collaborator

Description

Dataset.diff handled n > 1 by calling itself with n - 1, which goes back to its first version. Each order rebuilt the whole dataset and added a stack frame, so n close to the recursion limit raised a RecursionError.

  • Slice the coordinates and indexes once and only loop over the differences of the data variables (no recursion, ~2.5x faster for n=300)
  • The recursive call also did not pass on label, so label="lower" with n > 1 labelled the result like "upper" after the first step, e.g. [1, 2, 3, 4] instead of [0, 1, 2, 3] for n=2; this is fixed as well

Checklist

AI Disclosure

  • This PR contains AI-generated content.
    • I have tested any AI-generated content in my PR.
    • I take responsibility for any AI-generated content in my PR.
      Tools: Claude Code

[This is Claude Code on behalf of Michael Niklas]

🤖 Generated with Claude Code

headtr1ck and others added 2 commits October 9, 2026 22:55
@headtr1ck @claude
Loop instead of recursing in Dataset.diff
80a3531 
Dataset.diff called itself for every order n > 1, which raised a
RecursionError for n close to the recursion limit and rebuilt the whole
dataset for every order. Slice the coordinates once and only loop over
the differences of the data variables.

The recursive call also did not pass on the label, so label="lower" was
only used for the first difference.

Co-authored-by: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@headtr1ck @claude
Add PR number to whats-new
c6cb200 
Co-authored-by: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@headtr1ck headtr1ck added the plan to merge Final call for comments label Oct 10, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

plan to merge Final call for comments

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

diff raises RecursionError when n is around 1000 or more

1 participant

@headtr1ck