Skip to content
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

Support Jagged branch from CMS NanoAOD #22

Merged
merged 14 commits into from
Jul 3, 2021
Merged

Conversation

Moelf
Copy link
Member

@Moelf Moelf commented Jul 2, 2021

  • handle multiple blocks in datastream
  • hard code TLeafB

@codecov
Copy link

codecov bot commented Jul 2, 2021

Codecov Report

Merging #22 (56a13e9) into master (67f1e36) will increase coverage by 2.78%.
The diff coverage is 92.06%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #22      +/-   ##
==========================================
+ Coverage   78.09%   80.88%   +2.78%     
==========================================
  Files           7        8       +1     
  Lines         904      926      +22     
==========================================
+ Hits          706      749      +43     
+ Misses        198      177      -21     
Impacted Files Coverage Δ
src/custom.jl 0.00% <0.00%> (ø)
src/bootstrap.jl 79.40% <91.66%> (+8.81%) ⬆️
src/root.jl 79.37% <92.85%> (+0.02%) ⬆️
src/UnROOT.jl 100.00% <100.00%> (ø)
src/io.jl 89.39% <100.00%> (+2.82%) ⬆️
src/streamers.jl 86.79% <100.00%> (+0.13%) ⬆️
src/types.jl 93.50% <100.00%> (+0.08%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 67f1e36...56a13e9. Read the comment docs.

@Moelf Moelf marked this pull request as draft July 2, 2021 10:00
@Moelf Moelf changed the title [WIP] Support Jagged branch [WIP] Support Jagged branch from NanoAOD Jul 2, 2021
@Moelf
Copy link
Member Author

Moelf commented Jul 3, 2021

julia> using UnROOT

julia> array(ROOTFile("./test/samples/tree_with_jagged_array.root"), "t1/int32_array")
100-element Vector{Vector{Int32}}:
 []
 [0]
 [0, 1]
 [0, 1, 2]

@Moelf Moelf marked this pull request as ready for review July 3, 2021 18:48
@Moelf Moelf changed the title [WIP] Support Jagged branch from NanoAOD Support Jagged branch from CMS's NanoAOD Jul 3, 2021
@Moelf Moelf changed the title Support Jagged branch from CMS's NanoAOD Support Jagged branch from CMS NanoAOD Jul 3, 2021
@Moelf
Copy link
Member Author

Moelf commented Jul 3, 2021

@tamasgal I'm gonna squash merge this since tests and converge both seem happy. I'd propose drop 1.0 support since next LTS (1.6 or 1.7) is right around the corner. Let me know what you think

@tamasgal
Copy link
Member

tamasgal commented Jul 3, 2021

Very nice work!

I'd suggest to add hasproperty since this seems to be the only requirement we need and it's a one-liner: https://github.com/JuliaLang/julia/blob/6aaedecc447e3d8226d5027fb13d0c3cbfbfea2a/base/reflection.jl#L1583-L1590

Something like below right into UnROOT.jl or in a separate file compat.jl:

if VERSION < v"1.2"
    hasproperty(x, s::Symbol) = s in fieldnames(typeof(x))
end

@Moelf Moelf force-pushed the jagged_task branch 2 times, most recently from 5e9146f to ea7a5dc Compare July 3, 2021 21:45
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.

None yet

2 participants