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

Alternative to root_numpy #404

Closed
cbrisboi opened this issue Jan 8, 2021 · 11 comments
Closed

Alternative to root_numpy #404

cbrisboi opened this issue Jan 8, 2021 · 11 comments

Comments

@cbrisboi
Copy link
Contributor

cbrisboi commented Jan 8, 2021

I was looking at updating hawc_hal to use uproot rather than the old ROOT interface for its .root file io and saw that a big part of the root_numpy dependency is carried over from 3ML via the dependency on threeML/io/cern_root_utils. I would generally prefer to stay consistent with file handling between 3ML and hawc_hal. Is there a preference for uproot vs the new ROOT interface? This is already an issue in hawc_hal, see threeML/hawc_hal#42

The code looks straightforward to update, but before I run off and implement something that is not preferred, I thought asking would be wise first.

@grburgess
Copy link
Collaborator

@cbrisboi I think the other part of ROOT that is used in in the ROOT minimizers. Is this also a simple fix?

@cbrisboi
Copy link
Contributor Author

cbrisboi commented Jan 8, 2021

Since root_numpy is only for file io, this would be (to my mind) somewhat separate from the minimizers. It appears that in the threeML repo(rather than hawc_hal), the only place this code is used is in the VERITASLike plugin, although I am not taking responsibility for that plugin, as I am not a member of VERITAS.

@grburgess
Copy link
Collaborator

@cbrisboi
Copy link
Contributor Author

cbrisboi commented Jan 8, 2021

No, thats using ROOT, rather than root_numpy
See https://github.com/threeML/threeML/blob/master/threeML/io/cern_root_utils/tobject_to_numpy.py#L2

@grburgess
Copy link
Collaborator

Aha! Well, I'm all fine with it, but @ndilalla just did all the root stuff, so he should chime in.

@ndilalla
Copy link
Contributor

I am all in for getting rid of root_numpy and using uproot. @cbrisboi do you know which requirements this new package has?

@cbrisboi
Copy link
Contributor Author

cbrisboi commented Jan 11, 2021

On github, the only strict dependency is numpy, although if going through conda, which I suspect is preferred, the dependencies are ( using conda info uproot=4.0.0 and then copying the dependencies for python 3.7):

Here is the relevant github page https://github.com/scikit-hep/uproot4

dependencies:
    awkward >=1
    lz4
    python >=3.7,<3.8.0a0
    python-xxhash
    python_abi 3.7.* *_cp37m
    uproot-base
    xrootd
    zstandard

@grburgess
Copy link
Collaborator

Wow. This package is amazing.

@github-actions
Copy link

This issue has become stale. Is there an update? We will close in 15 days

@grburgess
Copy link
Collaborator

@cbrisboi will there be any progress on this? Otherwise, we need to go ahead and close or assign someone else.

@github-actions
Copy link

github-actions bot commented May 6, 2021

This issue has become stale. Is there an update? We will close in 15 days

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants