Skip to content
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

Make the default classes immutable and mark mutable classes explictly #205

Merged
merged 11 commits into from
Dec 3, 2021

Conversation

tmadlener
Copy link
Collaborator

@tmadlener tmadlener commented Aug 11, 2021

BEGINRELEASENOTES

  • Make the default classes immutable and mark mutable classes explictly via their class name (e.g. Hit and MutableHit). See a brief discussion in Proposal to make the Const classes default and explicitly mark the mutable ones as mutable #204 for more details on the reasons for this breaking change.
  • After these changes collections return mutable objects via their create functionality, and will only give access to the default (immutable) objects when they are const (e.g. when they are read from file).
  • In general these changes should make it easier for users to write interface that behave as expected, and also make it very obvious where objects are actually mutated already from looking at an interface definition.

ENDRELEASENOTES

This is a first shot to implement the proposal #204. As a first draft implementation it prepends Mutable to all class names that are mutable and removes the Const prefix from all immutable ones.

Built on top of #143!

  • Documentation
  • More detailed release notes
  • Check how much this breaks EDM4hep and other Key4hep dependencies
  • Also rename ConstExtraCode and ExtraCode?

@tmadlener tmadlener marked this pull request as draft August 11, 2021 15:09
@tmadlener
Copy link
Collaborator Author

Changes to EDM4hep itself amounts to only very few fixes with the current version: https://github.com/tmadlener/EDM4hep/compare/master...tmadlener:mutable-podio-classes?expand=1

@tmadlener
Copy link
Collaborator Author

A more realistic diff for usage where also mutable classes are actually used from k4SimDelphes:

tmadlener/k4SimDelphes@3d8f0f1

@tmadlener tmadlener force-pushed the mutable-classes branch 2 times, most recently from abde130 to 808ea39 Compare September 22, 2021 09:49
@tmadlener tmadlener changed the title [WIP] Make the default classes immutable and mark mutable classes explictly Make the default classes immutable and mark mutable classes explictly Sep 22, 2021
@tmadlener tmadlener marked this pull request as ready for review September 22, 2021 09:50
@tmadlener tmadlener force-pushed the mutable-classes branch 2 times, most recently from 08110fb to e4efbcb Compare October 11, 2021 16:43
@tmadlener tmadlener deleted the mutable-classes branch December 7, 2021 17:58
vvolkl added a commit to vvolkl/k4RecCalorimeter that referenced this pull request Feb 2, 2022
- Replace some dedicated types with `auto` and rename others (Cluster -> MutableCluster, ConstCluster -> Cluster) to adapt to the
  new naming scheme introduced in AIDASoft/podio#205.
- The necessary changes in EDM4hep: key4hep/EDM4hep#132 would break
  this otherwise. Due to the explict use of Mutable* (in attachCells)
  this now requires a new version of edm4hep.
vvolkl added a commit to vvolkl/k4SimGeant4 that referenced this pull request Feb 2, 2022
- Replace some dedicated types with  and rename others (Cluster -> MutableCluster, ConstCluster -> Cluster) to adapt to the new naming scheme introduced in AIDASoft/podio#205.
vvolkl added a commit to vvolkl/k4Gen that referenced this pull request Feb 2, 2022
- Replace some dedicated types with  and rename others (Cluster -> MutableCluster, ConstCluster -> Cluster) to adapt to the new naming scheme introduced in AIDASoft/podio#205.
vvolkl added a commit to vvolkl/CEPCSW that referenced this pull request Feb 2, 2022
- Replace some dedicated types with  and rename others (Cluster -> MutableCluster, ConstCluster -> Cluster) to adapt to the new naming scheme introduced in AIDASoft/podio#205.
vvolkl added a commit to vvolkl/dual-readout that referenced this pull request Feb 2, 2022
- Replace some dedicated types with  and rename others (Cluster -> MutableCluster, ConstCluster -> Cluster) to adapt to the new naming scheme introduced in AIDASoft/podio#205.
SanghyunKo pushed a commit to HEP-FCC/dual-readout that referenced this pull request Feb 3, 2022
- Replace some dedicated types with  and rename others (Cluster -> MutableCluster, ConstCluster -> Cluster) to adapt to the new naming scheme introduced in AIDASoft/podio#205.
vvolkl added a commit to HEP-FCC/k4RecCalorimeter that referenced this pull request Feb 3, 2022
* Adapt to new podio generated class names

- Replace some dedicated types with `auto` and rename others (Cluster -> MutableCluster, ConstCluster -> Cluster) to adapt to the
  new naming scheme introduced in AIDASoft/podio#205.
- The necessary changes in EDM4hep: key4hep/EDM4hep#132 would break
  this otherwise. Due to the explict use of Mutable* (in attachCells)
  this now requires a new version of edm4hep.

* Update RecCalorimeter/src/components/CorrectECalBarrelSliWinCluster.cpp

Co-authored-by: Brieuc Francois <brieuc.francois@cern.ch>

Co-authored-by: Brieuc Francois <brieuc.francois@cern.ch>
vvolkl added a commit to HEP-FCC/k4Gen that referenced this pull request Feb 4, 2022
- Replace some dedicated types with  and rename others (Cluster -> MutableCluster, ConstCluster -> Cluster) to adapt to the new naming scheme introduced in AIDASoft/podio#205.
vvolkl added a commit to HEP-FCC/k4SimGeant4 that referenced this pull request Feb 4, 2022
- Replace some dedicated types with  and rename others (Cluster -> MutableCluster, ConstCluster -> Cluster) to adapt to the new naming scheme introduced in AIDASoft/podio#205.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants