Remove support for legacy I/O components#475
Conversation
|
doctest workflows are broken also without this, see e.g. #476 key4hep nightlies on alma9 suffer from a missing fix in Gaudi |
tmadlener
force-pushed
the
rm-podio-data-svc
branch
from
67b9a3b to
0929b1b
Compare
|
This can probably be merged without a new tag, since the only changes since the last tag are enabling c++26 and some CI workflow changes. |
jmcarcell
left a comment
jmcarcell
left a comment
There was a problem hiding this comment.
Looks good to me, I grepped and couldn't find more places where there is something to remove.
|
Now we are stumbling over key4hep/k4FWCore#400 |
|
I should really fix the downstream build, because this is visible there but there is always some repo failing... |
tmadlener
force-pushed
the
rm-podio-data-svc
branch
from
114285a to
e7a8e63
Compare
|
I have picked up your changes from #478 (verbatim) and added similar changes to also set the particle id meta information to I have to try and find a test case that requires the pre-filling. |
|
@jmcarcell should we merge this to see if key4hep/k4FWCore#406 is necessary immediately or whether we might be able to push that for a bit? I am not sure if I can put together a test case that conclusively shows that pre-loading fixes potentially remaining issues. edit: The failing CIs are due to
|
|
Yes, I think that just putting in |
|
I think the failure in #478 might also have been that that only moves the cell id encoding parts, but the same pattern was used for the particle id meta stuff, so there was still another exception that wasn't addressed fully there. |
|
In the branch there it is commented out |
|
https://github.com/key4hep/k4MarlinWrapper/pull/478/changes (is github acting up again? I don't see it here). In any case, we pick it up here unless we want to split out those changes into a separate PR and make this about removing the legacy bits only? |
|
The last commit was in the branch but apparently because it was closed it never went through, weird because I remember testing this locally... Now we can see that #478 passes the Ubuntu pipeline. I've put in #478 the last commit of this branch so it's split so up to you; it's not a completely clean split since a variable that is used also for the PodioDataSvc branch also has to be removed. |
tmadlener
force-pushed
the
rm-podio-data-svc
branch
from
f061db3 to
8ecf42f
Compare
|
Split it off to have a slightly cleaner history to come back to in case it's necessary. I have also tested this locally with CLD and ILD reconstructions (again with LCIO and EDM4hep inputs). Both run and without crashes, so I think it should also be save to merge this (after a potential review) to unblock key4hep/k4FWCore#392 (depending on the timeline of that).
This is now no longer true! |
Don't use the to be deprecated DataHandle for the PodioDataSvc and instead store directly into format that is understood and usable via Functionals and IOSvc. Use new k4FWCore functionality to assign an id on the fly.
This breaks conversion using the PodioDataSvc since the MetadataSvc does not talk to that frame
Co-authored-by: Juan Miguel Carceller <22276694+jmcarcell@users.noreply.github.com>
Co-authored-by: Juan Miguel Carceller <22276694+jmcarcell@users.noreply.github.com>
jmcarcell
force-pushed
the
rm-podio-data-svc
branch
from
8ecf42f to
72e64d1
Compare
|
Looks good now and the Ubuntu CI is passing (Alma 9 doesn't work on LCG stacks until there is a new version of Gaudi). |
2 participants
BEGINRELEASENOTES
ENDRELEASENOTES
This is largely just removing things. I have moved the
getEDM4hepEventinto the commonStoreUtilssince we need this in both directions. Otherwise it's essentially just switching to newer utilities from k4FWCore and removing / changing tests that usedk4DataSvc.k4FWCore::(get|put)CellIDEncodingAlgTool)