Skip to content

fix: avoid recursion limit when concatenating EmptyArrays#3664

Merged
ianna merged 9 commits intoscikit-hep:mainfrom
ikrommyd:emptyarray-mergemany
Oct 17, 2025
Merged

fix: avoid recursion limit when concatenating EmptyArrays#3664
ianna merged 9 commits intoscikit-hep:mainfrom
ikrommyd:emptyarray-mergemany

Conversation

@ikrommyd
Copy link
Copy Markdown
Collaborator

@ikrommyd ikrommyd commented Sep 26, 2025

Fixes #3663

@codecov
Copy link
Copy Markdown

codecov bot commented Sep 26, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.66%. Comparing base (b749e49) to head (5ce6425).
⚠️ Report is 440 commits behind head on main.

Additional details and impacted files
Files with missing lines Coverage Δ
src/awkward/contents/emptyarray.py 76.47% <100.00%> (+0.84%) ⬆️

... and 196 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 26, 2025

The documentation preview is ready to be viewed at http://preview.awkward-array.org.s3-website.us-east-1.amazonaws.com/PR3664

@ikrommyd ikrommyd mentioned this pull request Sep 26, 2025
Closed
ikrommyd
ikrommyd commented Sep 26, 2025
View reviewed changes
ianna
ianna requested changes Sep 27, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@ianna ianna left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ikrommyd - thanks for looking into it! I wonder if it's better to do a lazy iteration instead of a list comprehension. Thanks.

@ikrommyd
Copy link
Copy Markdown
Collaborator Author

ikrommyd commented Sep 28, 2025

@ikrommyd - thanks for looking into it! I wonder if it's better to do a lazy iteration instead of a list comprehension. Thanks.

What lazy iteration did you think about? others is already a list in memory and I'm just trying to drop the empty array elements. What do you have in mind?

@ikrommyd ikrommyd requested a review from ianna October 1, 2025 23:30
@ianna ianna added the pr-next-release Required for the next release label Oct 16, 2025
ianna
ianna approved these changes Oct 17, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@ianna ianna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ikrommyd - Great! Thanks for fixing it. The tests pass - I'll enable auto-merge. Thanks!

@ianna ianna enabled auto-merge (squash) October 17, 2025 09:26
@ianna ianna merged commit c5a8fe6 into scikit-hep:main Oct 17, 2025
41 checks passed
@ikrommyd ikrommyd deleted the emptyarray-mergemany branch October 17, 2025 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ianna ianna ianna approved these changes

Assignees

No one assigned

Labels

pr-next-release Required for the next release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Concatenating arrays with EmptyArray in the layout hits the python recursion limit

2 participants

@ikrommyd @ianna