Add a new type to CxxUtils for arrays of optional values

This merge request adds a type to the CxxUtils package, the OptionalArray, which for a given template type T wraps the std::array<std::optional<T>> type. It adds several useful helper methods which make the type more ergonomic to use. This can be used later in applications such as the Trk::TrackSummary, or other places where an array of optional values is required.

I've done my best to add comments and tests for maintainability.

This merge request doesn't change any existing code, and should thus not break anything.

This merge request affects 1 package:

• Control/CxxUtils

Adding @ssnyder ,@rbianchi as watchers

added Core master review-pending-level-1 labels

CI Result FAILURE (hash 2e9a9602)

	Athena	AthSimulation	AthGeneration	AnalysisBase
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 1
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 20394]

Hey

Some points/question

• I guess it would be good to have a review expert

• I guess one should assume that this to be used generally outside the tracking realm and an the particular EDM applications to find its way to cxxUtils

• The general comment /question is you envision this like an "adaptor" / a set of "helper methods" (like imaging the *this aka std::array as arg to something like the coalesce) or as kind of container

  • Do you envision the main usage to be the method_x in which case why not just set of functions /helpers / alg and the need to hide a payload in a class e.g

 template<typename T, size_t N>
void coalesce_l(OptionalArray<T, N> & modify , const OptionalArray<T, N> & rhs);

• Do you envision it like a wrapper/adaptor class. In which case should one be able to get back the data after the operations? i.e if I pass my array of optional to this type and coalesce should I still be able to get it back and pass it to an std::algorithm, or another interface boundary expecting an std::array or proper type?

• Do you envision it as a stand alone container (no data() , no free helpers , as is now?) in which case do we need [] ,begin,end the main bits of the std::array container

Jenkins please retry a build

This merge request affects 1 package:

• Control/CxxUtils

Adding @ssnyder ,@rbianchi as watchers

CI Result SUCCESS (hash 2e9a9602)

	Athena	AthSimulation	AthGeneration	AnalysisBase
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 20446]

Escalating to L2 for a second opinion. On my side, the code looks well written and maintainable thanks to the tests. L1 reviewer.

added review-pending-level-2 label and removed review-pending-level-1 label

added review-pending-expert label and removed review-pending-level-2 label

I agree that code is well written and documented, but points raised by Christos are valid. I would ask for experts opinion, tagging @ssnyder , @wlampl and @emoyse .

Pavol [as L2 MR shifter]

I already made a number of comments on the code.

Sigh --- if you hit the wrong button, jira shows you the comments but doesn't show them to anyone else...

I guess is gitlab start review etc new buttons ?

Marking this as user action required now that an expert review has occured.

added review-user-action-required label and removed review-pending-expert label

added 2 commits

• bf68cc38 - Add a CxxUtils type for arrays of optionals
• d53e0526 - Add tests for new OptionalArray type

Compare with previous version

This merge request affects 1 package:

• Control/CxxUtils

Adding @ssnyder ,@rbianchi as watchers

added review-pending-level-1 label and removed review-user-action-required label

added 1 commit

• 370388fa - Add accessors to underlying containers

Compare with previous version

resolved all threads

This merge request affects 1 package:

• Control/CxxUtils

Adding @ssnyder ,@rbianchi as watchers

CI Result SUCCESS (hash d53e0526)

	Athena	AthSimulation	AthGeneration	AnalysisBase
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 21455]

CI Result SUCCESS (hash 370388fa)

	Athena	AthSimulation	AthGeneration	AnalysisBase
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 21457]

Looks good to me, adding review-pending-expert again to approve.

Cheers, Volker (L1)

added review-pending-expert label and removed review-pending-level-1 label

ping @ssnyder can you take another look (as I guess you are the expert here)

Looks good to me.

added review-approved label

merged

mentioned in commit 404bac6f

removed review-pending-expert label

added sweep:ignore label