JSON Exporter: Improvements

[kjvbrt]

BEGINRELEASENOTES

• JSON Exporter: Adding the possibility to provide list of events
• Update JSON Exporter to use Frame based I/O

ENDRELEASENOTES

[tmadlener]

Hi Juraj, apologies #185 leads to some merge conflicts here, because I clang-formated the json tool to make the pre-commit workflow pass. In principle, I think the easiest is to do the same on your end before trying to merge/rebase.

[kjvbrt]

Hello @tmadlener, merge resulted only in few conflicts after the clang-format :)

Can old edm4hep root files with "metadata" tree instead of "podio_metadata" tree be read somehow using frame?

[tmadlener]

Hi @kjvbrt, sorry about the delay. You can use the podio::ROOTLegacyReader to read older root files into the Frame world. Then you would not need to use the EventStore at all any longer.

The only caveat is that collection meta data is currently not supported there. I don't know if you need them.

[kjvbrt]

The test for the converter requires files to be created in frame compatible manner. Adapting the current write test breaks the following other tests:

      Start  1: write_events
 1/29 Test  #1: write_events ........................................   Passed    0.95 sec
      Start  2: read_events
 2/29 Test  #2: read_events .........................................***Failed    7.94 sec
      Start  3: write_events_sio
 3/29 Test  #3: write_events_sio ....................................   Passed    0.05 sec
      Start  4: read_events_sio
 4/29 Test  #4: read_events_sio .....................................Subprocess aborted***Exception:   0.09 sec
      Start  5: test_rdf
 5/29 Test  #5: test_rdf ............................................Subprocess aborted***Exception:   4.37 sec
      Start  6: py_test_rdf
 6/29 Test  #6: py_test_rdf .........................................***Failed  Error regular expression found in output. Regex=[error]  4.69 sec

[tmadlener]

#184 should in principle introduce Frame based I/O. Let me have a look at that again and potentially merge it

[tmadlener]

Now that #184 has been merged, this should also be able to use the test outputs again. It looks like there was no (or a different) .clang-format file present locally, since there are quite a few format issues flagged by pre-commit.

I have essentially just minor comments at this point, apart from the partially duplicated podio-dump functionality, which I don't think should be part of this program.

[hegner]

@kjvbrt - can you follow up with the clang format results on work on those?

[tmadlener]

Suggested change

	std::string evntStr;
	for (auto& evnt : eventVec) {
	evntStr += std::to_string(evnt);
	evntStr += ",";
	}
	evntStr.pop_back();
	std::cout << " " << evntStr << std::endl;
	std::cout << eventVec[0];
	for (size_t i = 1; i < eventVec.size(); ++i) {
	std::cout << "," << eventVec[i];
	}
	std::cout << std::endl;

Avoids copying and re-allocating a string each iteration. (Probably not really a problem here since this is only executed once).

[kjvbrt]

But what about trailing comma?

[tmadlener]

There is no trailing comma in this case, because we print the first element of the vector and then put a leading comma in front of all other elements.

[kjvbrt]

Yes :)

[tmadlener]

Is the [WIP] label still appropriate? Or is this in principle ready to be merged?