-
Notifications
You must be signed in to change notification settings - Fork 60
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
python/templates: add references between Objects and Collection types #465
Conversation
e7c1b4a
to
8cbadc2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this proposal. I think this is very useful and also necessary (e.g. for #257). So my main concern for this PR here is to make sure that we do this comprehensively and consistently (also taking into account other c++ conventions).
Overall podio itself does not (yet) have a convention (or at least not one that is followed consistently) for member types. Since some of the exiting member types try to mimic the ones from std::vector
for the collections, I think we should try to just continue there and try to follow the same convention of using <xyz>_type
to define member types.
Then one question is which member types do the different (user level) classes need?
Finally one that mainly concerns Jana2 and how it is used there; Are there any concerns wrt breaking things when we introduce this?
Good suggestion.
The only type reference that we were truly missing was from object to collection, because no object method or member would provide that. I don't see what else we could add, the Obj, Data and CollectionData are not part of the public interface.
Since this only adds fields, this should be backwards-compatible with any codebase. |
Thanks for address the comments. Could you rebase this onto the latest master/merge that in to pick up the updated CI configuration, please? |
1f26711
to
8506bc5
Compare
8506bc5
to
ad1160e
Compare
#1108) This takes advantage of AIDASoft/podio#465 to start deprecating one of the two responsibilities of the `make_datamodel_glue.py`. A similar change to JANA2 will be done independently. I suggest to keep the `#if` pragmas for now, in case we want to bisect possible issues with the latest PODIO.
BEGINRELEASENOTES
Object::collection_type
. Conversely, the object type is reachable asObjectCollection::value_type
. The mutable objects can be reached viaObject::mutable_type
.ENDRELEASENOTES
This should allow to remove some codegen hack from EICrecon https://github.com/eic/EICrecon/blob/1f6727948696b70fc29b1fe582f23f2e37c476b2/src/services/io/podio/make_datamodel_glue.py#L76-L84
cc @wdconinc @nathanwbrei