wrap AnaToolHandle around AsgToolConfig
The basic reasoning here is that AnaToolHandle is essentially the out-dated and discouraged mechanism for creating tools, and has been replaced with AsgToolConfig for quite a while (or alternatively indirectly via AnaAlgorithmConfig).
I did have to disable some Athena tests for setting sub-tools in Athena. There are already some of those disabled, and the basic logic is that since this is the legacy way of doing things, unless this breaks existing code (as verified by CI tests) I no longer want to support all the possible edge cases in this rather brittle and complex piece of code.
Merge request reports
Activity
added full-unit-tests label
Athena AthSimulation AthGeneration AnalysisBase AthAnalysis externals 
cmake make 
required tests 
optional tests Full details available on this CI monitor view
Hi @tadej,
Well, that was my first thought as well, but it is used in ~150 files within the athena repository itself, to say nothing about any other code bases like analysis frameworks. That's not prohibitive, but it seems like it would be quite a bit more laborious. Particularly since some users seem to do rather weird stuff, and they may be relying on some of the more counter-intuitive behavior of
AnaToolHandle. So this would likely not just be a straightforward update replacing pattern A with pattern B, but probably needs a bit more thought in each place its used.I mean it would probably still be worthwhile to do such a global update, but for now I'm more inclined to find a volunteer to do that instead of doing it myself. And in my experience such a migration is often easier if the class you want to replace is a not-too-complicated wrapper around the classes you want to replace it with. Otherwise such a migration may end up being more like a rewrite than an update. Plus it also allows a multi-step migration strategy in which you slowly strip the more awkward features and use cases, which can be easier in some cases.
The other question is also whether if you do such a migration you only want to do a "minimal" migration of replacing
AnaToolHandlewithAsgToolConfig. The alternative in many cases would be to say you just put in aToolHandleand do the tool configuration from the python side. The original motivation forAnaToolHandlewas that we did not have python configuration outside of athena. At the same time it is used in athena-only code as well, so apparently in some situations developers do prefer the C++ configuration over the python configuration. And it is also arguable whether we are completely ready for such a shift on the analysis side as well.Cheers, Nils
-
Athena AthSimulation AthGeneration AnalysisBase AthAnalysis externals cmake make required tests optional tests Full details available on this CI monitor view
added Analysis label
-
Athena AthSimulation AthGeneration AnalysisBase AthAnalysis externals cmake make required tests optional tests Full details available on this CI monitor view
-
Athena AthSimulation AthGeneration AnalysisBase AthAnalysis externals cmake make required tests optional tests Full details available on this CI monitor view
