-
Notifications
You must be signed in to change notification settings - Fork 31
Make it possible to keep or drop all collections of a given datatype #362
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
Conversation
|
This will indeed be very useful! |
eb97ce1 to
4a6db0f
Compare
|
I am fairly happy with the syntax / grammar for the output commands here. The main question I still have is whether it should be |
I would say, leave it as it is now, with the namespace. |
It's a bit confusing that the string contradicts what is actually tested.
The way we use it we call it once per collection. Additionally, when adding support for keeping and dropping based on types the caching starts to become more complicated.
Using "type" as second element of an output command will now make the KeepDropSwitch take decisions on types as well. The isOn command gets a second overload that takes a type name as well. Type names are matched exactly without support for wildcards. We keep the old isOn overload that only takes a single argument for easier testing without types and for some backwards compatibility.
This makes it possible to keep / drop collections based on type in addition to the existintg name based selection
Co-authored-by: Juan Miguel Carceller <[email protected]>
Co-authored-by: Juan Miguel Carceller <[email protected]>
43dd2bc to
cc2ab0a
Compare
|
I think now all comments should be addressed and this could be merged. |
BEGINRELEASENOTES
typesubcommand to theoutputCommandsgrammar to keep / drop all collections of a given datatype.ENDRELEASENOTES
This functionality was also present for LCIO / Marlin and I think it's quite useful, because it just allows one to write e.g.
drop type edm4hep::SimTrackerHitCollectionto drop all sim tracker hits without having to know all names first.I have implemented this here instead of podio, because I think it fits better here.
For now I have also kept the overload of
KeepDropSwitch::isOnthat only takes a single string, but I don't think there are any users outside for which backwards compatibility would be a concern (see also #351 where theKeepDropSwitchwas moved into thek4FWCorenamespace). Removing that would require touching the existing unit tests, but that would not be a real problem, I think.There is also some initial refactoring of the
KeepDropSwitchthat could in principle be pulled into a separate PR if necessary.