Reading of c-arrays + more customstructs

[briederer]

After prior discussion in #165 and #166 respectively I decided to put a little bit of effort into extending it to fix #9 finally.

For that I simply enhanced the customstruct to an arbitrary dimensional FixSizeArray and parse the fLeaf.fTitle for the decomposition. Everything else works as expected.

Two caveats:

1. Since all of it relies on wrapping the StaticArrays.SArray struct nicely we are also limited to there specifications. With the current implementation of the struct Base.sizeof() does not work out of the box due to a missing 4.th parameter in the SArray-type. (see: https://github.com/briederer/UnROOT.jl/blob/3785a042a9cd9855e5f0b9ee18e3b89448bb15d3/src/custom.jl#L49-L53). This means we either add an (for this purpose) unnecessary parameter to the struct or we simply overwrite the Base.sizeof method as is done here.
2. I've also added (like it is usually done in Julia) aliases for FixSizeVector and FixSizeMatrix to improve readability in the output. However due to some restrictions (see Type aliases are not printed as aliases if the aliased type is imported from a different module JuliaLang/julia#40448) the aliases would only be seen for the User if the additional types are exported. But this is not my choice 😉

Closes #9

[Moelf]

julia> a = @SMatrix [1 2 3; 4 5 6]
2×3 SMatrix{2, 3, Int64, 6} with indices SOneTo(2)×SOneTo(3):
 1  2  3
 4  5  6

julia> sizeof(a)
48

julia> sizeof(typeof(a))
48

it should work?

[Moelf]

I forgot why we can't use SArray directly actually, I read the last discussion, it seems to be because we build the buffer by calling the constructor:

UnROOT.jl/src/iteration.jl

Line 116 in 7d37122

_buffer = VectorOfVectors(T(), Int32[1])

and of course the StaticArray doesn't have constructor. But we can work around this by defining our own:

_initialize(::Type{T}) = T()
_initialize(::Type{T<:SArray}) = zero(T)

this way we don't have to wrap them?

[briederer]

Indeed in this case it works. But this is due to the fact, that the actual type of your SMatrix is SMatrix{2, 3, Int64, 6} which is an alias for SArray{Tuple{2, 3}, Int64, 2, 6}.
However the inner type in "my" FixSizeArray is missing the last entry in the type arguments (i.e. it is SArray{Tuple{...}, T, 2} which is basically a subtype since the last argument can be deduced from the product of all Tuple{...} entries. So in fact the wrapped struct is not exactly like in your example.
However it could be adapted by adding everywhere this 4th argument, which seems unnecessary to me though.

[briederer]

I forgot why we can't use SArray directly actually, I read the last discussion, it seems to be because we build the buffer by calling the constructor:

UnROOT.jl/src/iteration.jl

Line 116 in 7d37122

_buffer = VectorOfVectors(T(), Int32[1])

and of course the StaticArray doesn't have constructor. But we can work around this by defining our own:

_initialize(::Type{T}) = T()
_initialize(::Type{T<:SArray}) = zero(T)

this way we don't have to wrap them?

I am not sure this would work?
First problem is that according to the last discussion the arrays are Nojagg and thus would not even enter this part of the code. However even if you circumvent this problem I am not sure if calling a VectorOfVectors(StaticArrays(),...) wouldn't be a bit of an overkill (too many nested vector-like structs).
But I agree that getting rid off the wrapper would be a good thing.

[briederer]

I've added now also the possibility to allwo the customstructs dict to allow for special types simply by branchnames.

This enables a bit more customizability if someone needs special interpretation of some branches.
E.g. i've got one branch called complex[2]/D containing complex numbers stored as two entries. Since it is not a C++-class the branch has no fClassName field and the usual approach does not work. Therefore searching for names too is a possible workaround.

[briederer]

To be honest I just realised that the last addition is everything that I need for any of my "special branches".

So if you think that the other parts of this PR (and what has been in added in #166) adds too much overhead/unnecessary stuff I would be totally fine with removing all of that. As long as defining "customstructs" also via fTitle (or fName which is probably more intuitive) remains in this PR I am happy 😊

[briederer]

Btw. why are tests failing?
I can't access the CI-logs so I don't now what's happening there. Will check if tests fail locally too, tomorrow.

[Moelf]

https://github.com/JuliaHEP/UnROOT.jl/runs/6309779626?check_suite_focus=true

is log somehow not visible?

[Moelf]

does test pass for you locally? ]test UnROOT

[briederer]

Well they are accessible but it says "The log was not found" 🤔

[briederer]

does test pass for you locally? ]test UnROOT

Will try it tomorrow.

[briederer]

Tried now the tests locally. The failing one is this:

UnROOT.jl/test/runtests.jl

Lines 733 to 738 in 7d37122

	@testset "SourceStream remote" begin
	t = LazyTree("https://scikit-hep.org/uproot3/examples/Zmumu.root", "events")
	@test t.eta1[1] ≈ -1.21769
	@test t.eta1[end] ≈ -1.57044
	show(devnull, t) # test display
	end

It seems that this file has a TBranch called "Type" populated with TLeafC which is a variable size string. Problem that occurs is that this has a leaf.fLen > 1 and thus enters the condition added in #166. Before my current modification (i.e. extending to arbitrary size arrays) the test did work because it simply reinterpreted the string wrong (as a FixLenVector instead of a string) which is not covered by the tests.

Fixed it by simply leaving it as FixSizeVector{leaf.fLen,UInt8} which is a "untranslated" vector of chars. I didn't manage to adopt the output to produce a string for each event (problem with no definite size-strings when reinterpreting) but I guess this could be adapted. I.e. it simply would need another wrapping before being printed like String.(FixSizeVector{leaf.fLen,UInt8})

[codecov]

Codecov Report

Merging #167 (2e2b444) into master (7d37122) will decrease coverage by 0.41%.
The diff coverage is 95.83%.

@@            Coverage Diff             @@
##           master     #167      +/-   ##
==========================================
- Coverage   90.71%   90.29%   -0.42%     
==========================================
  Files          11       11              
  Lines        1561     1566       +5     
==========================================
- Hits         1416     1414       -2     
- Misses        145      152       +7     

Impacted Files	Coverage Δ
src/UnROOT.jl	66.66% <ø> (-23.34%)	⬇️
src/custom.jl	97.89% <92.30%> (+0.02%)	⬆️
src/iteration.jl	88.94% <100.00%> (ø)
src/root.jl	93.82% <100.00%> (+0.02%)	⬆️
src/utils.jl	100.00% <100.00%> (ø)
src/streamers.jl	89.83% <0.00%> (-1.70%)	⬇️

---

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 7d37122...2e2b444. Read the comment docs.

[Moelf]

looks like it's passing for >=1.6, I will give it a read later and see if we can manage without so much custom type

[Moelf]

this is partly addressed in #188

[briederer]

Sadly it appears I'll never find time to finish this and (probably) won't need it any time in the near future.
Therefore I am closing the PR now.
In any case someone wants to pick it up again, feel free. I will not delete the branch.