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

fix: add support for Long64_t #2023

Merged
merged 4 commits into from
Dec 21, 2022
Merged

Conversation

ianna
Copy link
Collaborator

@ianna ianna commented Dec 20, 2022

No description provided.

@ianna ianna linked an issue Dec 20, 2022 that may be closed by this pull request
@ianna ianna changed the title fix: add test fix: add support for Long64_t Dec 20, 2022
@ianna ianna marked this pull request as draft December 20, 2022 09:05
@ianna ianna temporarily deployed to docs-preview December 20, 2022 09:17 — with GitHub Actions Inactive
@codecov
Copy link

codecov bot commented Dec 20, 2022

Codecov Report

Merging #2023 (87f9276) into main (b83d9dc) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

@ianna ianna temporarily deployed to docs-preview December 20, 2022 14:31 — with GitHub Actions Inactive
@ianna ianna temporarily deployed to docs-preview December 20, 2022 14:46 — with GitHub Actions Inactive
@ianna ianna marked this pull request as ready for review December 20, 2022 16:32
@ianna ianna requested a review from jpivarski December 20, 2022 16:32
@ianna ianna temporarily deployed to docs-preview December 20, 2022 16:43 — with GitHub Actions Inactive
Copy link
Member

@jpivarski jpivarski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it would work! @miranov25, are you able to test from this git branch or do we need to make an Uproot release for testing?

@miranov25
Copy link

@jpivarski, @ianna, Thank you. I am packing now. Tomorrow morning we will leave for our holiday. I quickly checked your branch, but I have some problems I installed via pip from the GitHub branch. I assume that was ok.
I assume some headed file is missing (something similar we had 4in September test):

git log . 
commit 87f9276a09abbb67e6df4582a6e71985b90f09e3 (HEAD -> main, tag: list, origin/2018-from_rdataframe-doesnt-support-long64_t)
Author: Ianna Osborne <ianna.osborne@cern.ch>
Date:   Tue Dec 20 17:27:11 2022 +0100

    fix: specialize type_to_name
(venv3) Singularity> pip install /u/miranov/github/awkward/ 
Processing /u/miranov/github/awkward
(venv3) Singularity> pip show awkward
Name: awkward
Version: 2.0.2

I cannot rule out the possibility that I have done something wrong. Next time I will be able to check in 3-5 days.

Error message/stack trace

In [6]:     if doTest:
   ...:         rdfTest=rdf1.Range(0,10)
   ...:         rdfTest.Snapshot("testVarRDF","testVarRDF.root", varList)
   ...:         array = ak.from_rdataframe(rdfTest, columns=varList)
   ...:         df=ak.to_dataframe(array)
   ...:         print(df.head(5),df.shape)
   ...:     #
   ...: 
---------------------------------------------------------------------------
OSError                                   Traceback (most recent call last)
Input In [6], in <cell line: 1>()
      2 rdfTest=rdf1.Range(0,10)
      3 rdfTest.Snapshot("testVarRDF","testVarRDF.root", varList)
----> 4 array = ak.from_rdataframe(rdfTest, columns=varList)
      5 df=ak.to_dataframe(array)
      6 print(df.head(5),df.shape)

File /venv/venv3/lib/python3.8/site-packages/awkward/operations/ak_from_rdataframe.py:28, in from_rdataframe(rdf, columns)
      7 """
      8 Args:
      9     rdf (`ROOT.RDataFrame`): ROOT RDataFrame to convert into an
   (...)
     23 See also #ak.to_rdataframe.
     24 """
     25 with ak._errors.OperationErrorContext(
     26     "ak.from_rdataframe", dict(rdf=rdf, columns=columns)
     27 ):
---> 28     return _impl(rdf, columns)

File /venv/venv3/lib/python3.8/site-packages/awkward/operations/ak_from_rdataframe.py:32, in _impl(data_frame, columns)
     31 def _impl(data_frame, columns):
---> 32     import awkward._connect.rdataframe.from_rdataframe  # noqa: F401
     34     if isinstance(columns, str):
     35         columns = (columns,)

File /alicesw3b/sw/ubuntu2004_x86-64/ROOT/v6-26-04-patches-alice1-local1/lib/ROOT/_facade.py:153, in ROOTFacade._set_import_hook.<locals>._importhook(name, *args, **kwds)
    151     except Exception:
    152         pass
--> 153 return _orig_ihook(name, *args, **kwds)

File /venv/venv3/lib/python3.8/site-packages/awkward/_connect/rdataframe/from_rdataframe.py:44, in <module>
     33 numpy = ak._nplikes.Numpy.instance()
     36 cppyy.add_include_path(
     37     os.path.abspath(
     38         os.path.join(
   (...)
     42     )
     43 )
---> 44 cppyy.add_include_path(
     45     os.path.abspath(
     46         os.path.join(
     47             os.path.dirname(os.path.realpath(__file__)),
     48             os.path.pardir,
     49             "header-only",
     50         )
     51     )
     52 )
     53 compiler = ROOT.gInterpreter.Declare
     56 done = compiler(
     57     """
     58 #include "rdataframe/jagged_builders.h"
     59 """
     60 )

File /alicesw3b/sw/ubuntu2004_x86-64/ROOT/v6-26-04-patches-alice1-local1/lib/cppyy/__init__.py:221, in add_include_path(path)
    219 """Add a path to the include paths available to Cling."""
    220 if not os.path.isdir(path):
--> 221     raise OSError("no such directory: %s" % path)
    222 gbl.gInterpreter.AddIncludePath(path)

OSError: no such directory: /venv/venv3/lib/python3.8/site-packages/awkward/_connect/header-only

@agoose77
Copy link
Collaborator

@miranov25 installing directly from Git via pip is not supported. Instead, you need to clone the repo before running a prepare script. E.g., to build the awkward packages from Git into a wheels directory

this_path=$PWD
pushd $(mktemp -d)

git clone --depth=1 --recurse-submodules --shallow-submodules https://github.com/scikit-hep/awkward.git
cd awkward

pipx run nox -s prepare

mkdir -p "$this_path/wheels" 
pipx run  --python $(which python3) build -w -o "$this_path/wheels" awkward-cpp
pipx run  --python $(which python3) build -w -o "$this_path/wheels" .

popd

@ianna ianna merged commit 5c5b81e into main Dec 21, 2022
@ianna ianna deleted the 2018-from_rdataframe-doesnt-support-long64_t branch December 21, 2022 09:23
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.

from_rdataframe doesn't support Long64_t
4 participants