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

Make branch names for relations more legible for ROOT based I/O #405

Merged
merged 9 commits into from
Jun 5, 2023

Conversation

tmadlener
Copy link
Collaborator

@tmadlener tmadlener commented Apr 13, 2023

BEGINRELEASENOTES

  • Make the branch names for relations and vector members more legible and valid c++ variable names to improve interoperability with RDataFrame. Fixes Possibility of more intelligible branch names for relations #169
    • The branch names will have the following structure: _<collection-name>_<relation-name>, resp. _<collection-name>_<vectormember-name>, where relation-name, resp.vectormember-name are taken from the YAML definitions.
    • Subset collections will have a single branch with <collection-name>_objIdx. This makes it easier to disambiguate them from normal collections.
  • This is a breaking change if you use the root files directly! If you use the podio Readers/Writers everything should be transparent

ENDRELEASENOTES

@tmadlener tmadlener force-pushed the legible-branch-names branch 2 times, most recently from 5b6a981 to 91b727f Compare April 13, 2023 15:19
@tmadlener tmadlener changed the title [WIP] Make branch names for relations more legible for ROOT based I/O Make branch names for relations more legible for ROOT based I/O Apr 13, 2023
@hegner
Copy link
Collaborator

hegner commented May 25, 2023

@tmadlener - what is the status about the testing and the test file?

@tmadlener
Copy link
Collaborator Author

I have to figure out who has the rights to upload things to where we already host a legacy test file:

wget https://key4hep.web.cern.ch:443/testFiles/podio/v00-13/example.root > /dev/null 2>&1

After that it is hopefully just placing a file produced with a previous version of podio there and integrating things into the tests.

@andresailer
Copy link
Member

I have to figure out who has the rights to upload things to where we already host a legacy test file:

Soon you should be able to...

@tmadlener tmadlener force-pushed the legible-branch-names branch 2 times, most recently from ee1655e to 42f79a7 Compare May 31, 2023 09:14
hegner pushed a commit that referenced this pull request Jul 11, 2023
* Add a RNTuple writer

* Cleanup and add a reader

* Add compilation instructions for RNTuple

* Add tests

* Fix the reader and writer so that they pass most of the tests

* Commit missing changes in the header

* Add support for Generic Parameters

* Add an ugly workaround to the unique_ptr issue

* Read also vector members and remove some comments

* Do a bit of cleanup

* Do more cleanup, also compiler warnings

* Add names in rootUtils.h, fix a few compiler warnings

* Add a few minor changes

* Add missing changes in the headers

* Change map -> unordered_map and use append in CMakeLists.txt

* Simplify writing and reading of generic parameters

* Only create the ID table once

* Add CollectionInfo structs

* Add a ROOT version check

* Add missing endif()

* Add Name at the end of some names

* Add missing Name at the end

* Cast to rvalue

* Cache entries and reserve

* Add comment and remove old comments

* Remove a few couts

* Remove intermediate variables and use std::move

* Run clang-format

* Use clang-format on tests too

* Enable RNTuple I/O in Key4hep CI

* Check if dev3 workflows come with recent enough ROOT

* Change MakeField to the new signature

* Update the RNTuple reader and writer to use the buffer factory

* Run clang-format

* Update the RNTuple writer to use a bare model

* Add friends for Generic Parameters

* Update changes after the changes in the collectionID and string_view

* Run clang-format

* Update the reader and writer to conform to #405

* Reorganize and clean up code in the reader

* Run clang-format

* Simplify how the references are filled

---------

Co-authored-by: jmcarcell <jmcarcell@users.noreply.github.com>
Co-authored-by: tmadlener <thomas.madlener@desy.de>
hegner pushed a commit to hegner/podio that referenced this pull request Jul 27, 2023
* Add a RNTuple writer

* Cleanup and add a reader

* Add compilation instructions for RNTuple

* Add tests

* Fix the reader and writer so that they pass most of the tests

* Commit missing changes in the header

* Add support for Generic Parameters

* Add an ugly workaround to the unique_ptr issue

* Read also vector members and remove some comments

* Do a bit of cleanup

* Do more cleanup, also compiler warnings

* Add names in rootUtils.h, fix a few compiler warnings

* Add a few minor changes

* Add missing changes in the headers

* Change map -> unordered_map and use append in CMakeLists.txt

* Simplify writing and reading of generic parameters

* Only create the ID table once

* Add CollectionInfo structs

* Add a ROOT version check

* Add missing endif()

* Add Name at the end of some names

* Add missing Name at the end

* Cast to rvalue

* Cache entries and reserve

* Add comment and remove old comments

* Remove a few couts

* Remove intermediate variables and use std::move

* Run clang-format

* Use clang-format on tests too

* Enable RNTuple I/O in Key4hep CI

* Check if dev3 workflows come with recent enough ROOT

* Change MakeField to the new signature

* Update the RNTuple reader and writer to use the buffer factory

* Run clang-format

* Update the RNTuple writer to use a bare model

* Add friends for Generic Parameters

* Update changes after the changes in the collectionID and string_view

* Run clang-format

* Update the reader and writer to conform to AIDASoft#405

* Reorganize and clean up code in the reader

* Run clang-format

* Simplify how the references are filled

---------

Co-authored-by: jmcarcell <jmcarcell@users.noreply.github.com>
Co-authored-by: tmadlener <thomas.madlener@desy.de>
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.

Possibility of more intelligible branch names for relations
3 participants