Skip to content

fix: FreeSegments end calculation when crossing 2 GB boundary#1622

Merged
ianna merged 5 commits into
mainfrom
ariostas/fix_extend
Apr 30, 2026
Merged

fix: FreeSegments end calculation when crossing 2 GB boundary#1622
ianna merged 5 commits into
mainfrom
ariostas/fix_extend

Conversation

@ariostas

Copy link
Copy Markdown
Member

This PR fixes #1620. It turned out that #1601 caused an issue where self._data.end was set expecting the file to stay below 2GB, but then when writing it would realize it needed to switch to using a big file pointer, so there was a mismatch. The required_end takes into account whether it will have to be extended into a big file.

AI disclosure: I used assistance from Gemini for this PR, but I fully verified the changes and the logic.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes a regression in the ROOT writer’s FreeSegments bookkeeping when the file grows past the 2 GB “big file” boundary, preventing an at_end assertion failure during large extend() writes.

Changes:

  • Add FreeSegmentsData.required_end(...) to compute the correct logical end-of-file while accounting for small vs big TFree record sizing.
  • Update FreeSegments.allocate(...) and FreeSegments.release(...) to use required_end(...) instead of deriving end from potentially mismatched cached/allocation-derived sizes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/uproot/writing/_cascade.py
Comment thread src/uproot/writing/_cascade.py Outdated
@codecov

codecov Bot commented Apr 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.90909% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.76%. Comparing base (f4daa7a) to head (c78a813).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/uproot/writing/_cascade.py 90.90% 1 Missing and 1 partial ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
src/uproot/writing/_cascade.py 86.77% <90.90%> (+0.45%) ⬆️

@ariostas ariostas requested a review from ianna April 22, 2026 13:32

@ianna ianna left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@ariostas - Great! Thanks for fixing it. The coverage target is high - which is the only thing that is not green. I'll merge it. Thanks.

@ianna ianna enabled auto-merge (squash) April 30, 2026 08:19
@ianna ianna merged commit 30f70b0 into main Apr 30, 2026
23 of 24 checks passed
@ianna ianna deleted the ariostas/fix_extend branch April 30, 2026 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extending tree beyond 1.86 GB causes exception since v5.7.3

3 participants