search

scikit-hep / uproot5

ROOT I/O in pure Python and NumPy.
https://uproot.readthedocs.io
BSD 3-Clause "New" or "Revised" License
239 stars 76 forks source link

`xxhash` is currently a strict dependency for reading RNTuples #1271

Closed ariostas closed 2 months ago

ariostas commented 3 months ago

xxhash is currently a strict dependency for reading RNTuples even though it's not listed on pyproject.toml. The dependent code is here:

https://github.com/scikit-hep/uproot5/blob/ba3269b31cd791589bbd3f6d4634dbe606b9f997/src/uproot/compression.py#L433-L439

which could simply be removed to remove the dependency.

For writing, there will be a strict dependency and there is no way to get around it, but maybe we can make uproot.extras.xxhash pick between xxhash and ppxxh (which is pure Python, so it works in WASM).

jpivarski commented 3 months ago

Is the xxhash algorithm available in cramjam, which is one of Uproot's strict requirements?

Darn, it's not: https://github.com/milesgranger/cramjam/issues/147

But since xxhash is in Pyodide now (https://github.com/ifduyue/python-xxhash/issues/65), we can make xxhash a strict requirement for Uproot without causing installation issues for any use-cases that I can think of.

When that goes in (when xxhash becomes a strict requirement for Uproot), we'll need a new minor version number, 5.4.0.

jpivarski commented 3 months ago

(I'd rather not switch between libraries like xxhash and ppxxh implicitly because if there's a bug in one of them, it would be hard to reproduce. Two people could install Uproot the same way and get different libraries, one with the bug and one without. Understanding why that's happening—if we don't remember that uproot.extras.xxhash performs that switch—would be hard.)

ariostas commented 3 months ago

Oh interesting, I didn't realize that xxhash was now on Pyodide. That's pretty convenient.