-
Notifications
You must be signed in to change notification settings - Fork 19
Add Int conversion for string lengths, document embedded basket limitation #391
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
wtf 😆 |
|
If this somehow works out or at least leads to the right direction, I will definitely be amazed. On the other hand, it requires a major rework of how baskets are managed, spanning across a couple of fairly complex function. I think the chances are extremely low that something useful comes out but let's see 😉 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #391 +/- ##
==========================================
- Coverage 84.08% 83.74% -0.34%
==========================================
Files 21 21
Lines 3097 3100 +3
==========================================
- Hits 2604 2596 -8
- Misses 493 504 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Moelf <[email protected]>
Co-authored-by: Moelf <[email protected]>
Co-authored-by: Moelf <[email protected]>
|
nah, this looks too stupid of a fix to just work right? |
|
Yes it's on the wrong track. I figured that when I parse 90 bytes less of the |
|
Maybe it would help if I commit my additions and add a failing test with the sample file 😉 But I have no idea how this works here... |
|
It was 19 bytes: #389 (comment) I was also confused by those 19 bytes because I could not figure out where it could come from. 18 bytes + speedbump maybe? No idea 🙈 If I recall correctly it was right after parsing the leaves Line 625 in 310d987
TObjArray deserialisation logic, but I was stuck.
|
ROOT files with embedded baskets (from premature closure) fail with EOF errors. Embedded baskets have
fNbytes <= fKeylenand use inline data layout that differs from normal recovered baskets.Changes
Type safety improvement (
src/io.jl):Int()conversion when reading string lengths to match uproot5's NumPy 2.0 compatibility fixCode documentation (
src/bootstrap.jl):fNbytes <= fKeylen)Test documentation (
test/issues.jl):Current Status
Files with embedded baskets still fail - full support requires understanding ROOT's inline data layout and handling preamble byte count expectations. Users should reprocess such files through ROOT to normalize basket format, or use uproot (Python) which has full support.
Example
Reference: uproot5 commit b9a2a02,
TBasket.pylines 252-287Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
eospublic.cern.chappend!(empty!(Base.DL_LOAD_PATH), String[])
cd("/home/REDACTED/work/UnROOT.jl/UnROOT.jl/test")
append!(empty!(ARGS), String[])
include("/home/REDACTED/work/UnROOT.jl/UnROOT.jl/test/runtests.jl")` (dns block)
Original prompt
This section details on the original issue you should resolve
<issue_title>EOF when trying to open a file</issue_title>
<issue_description>Hi, I have a fairly simple ROOT file produced from Geant4 output (should not really matter I guess):[
MuonSimulationData.zip
I am able to read it with
uproot(and directly with ROOT reader) and its structure is:However, with UnROOT.jl I get:
I tried using the LazyTree but with no success either:
</issue_description>
<agent_instructions>when necessary, check how https://github.com/scikit-hep/uproot5/ handles this for reference</agent_instructions>
Comments on the Issue (you are @copilot in this section)
@Moelf > T=MuonEntry, # Explicitly tell UnROOT to treat each entry as a MuonEntrythis seems like a hallucination btw</comment_new>
<comment_new>@tamasgal
Hm, looks like fun... It's about recovered baskets which are still a bit foggy