feat: support reading of custom-length RNTuple floats and suppressed columns #1347
Conversation
|
It ended up only taking a few lines to support suppressed columns. I'll improve the reading of floats and add some tests. |
ariostas
changed the title
|
This PR is ready for review, but it needs scikit-hep/scikit-hep-testdata#167 for the tests to pass. Also, since there is still an issue with Dask, we'll just have to wait until that is resolved. |
|
@jpivarski I fixed the issue with Numpy 1 and I improved the tests. I still had to use |
There was a problem hiding this comment.
It's fine for comparisons with ROOT to involve isclose with tight tolerances (around the scale of numerical precision). What I meant was that if you know you'll be truncating at the Nth digit, it's better to test against an expected value that's truncated at the Nth digit, with isclose picking up any small errors, rather than using isclose to check for differences with respect to the original value. My reasoning was just that the Nth digit might be a huge error and leaving a window wide open like that could fail to identify some errors.
As a whole, this PR looks great and I'd say it's ready to be merged. This is the PR that brings RNTuple-reading up to 100% coverage, right?
Yes, as far as I can tell, after this PR we'll have 100% coverage of the current spec. |
This PR implements the reading of
Real32TruncandReal32Quantfields, which have a variable number of bits in the ranges 10-31 and 1-32, respectively.It also adds support for suppressed columns, at least in simple cases.