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

PodioInput: Read all collections by default, allow to limit them with the collections property #162

Merged
merged 4 commits into from
Dec 4, 2023

Conversation

tmadlener
Copy link
Contributor

@tmadlener tmadlener commented Nov 17, 2023

BEGINRELEASENOTES

  • Make the PodioInput read all collections by default, instead of requiring a full list of collections in the python config.
    • Keep the collections property to allow limiting the collections that are read to a subset of collections.
  • Avoid crashing with a seg-fault in case a non-existent collection is requested and instead emit a warning and continue.

ENDRELEASENOTES

Partially addresses #105

Comment on lines +196 to +213
const auto& collsToRead = [&]() {
if (m_collectionNames.empty()) {
return m_podioDataSvc->getEventFrame().getAvailableCollections();
} else {
return m_collectionNames.value();
}
}();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my c++ knowledge is not as advanced but why do we need this as a lambda? 🥴

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pattern is called an immediately invoked function expression (IIFE) and it is mainly used for "complicated" initialization logic, where it can help to

  • reduce the scope of temporary variables that are necessary for the initialization
  • Allows to declare the initialized object as const, instead of having it mutable and dealing with the logic outside.

So in this case really only to have a const& instead of just a &.

@tmadlener tmadlener force-pushed the improve-podio-input branch 2 times, most recently from 0b7e4b9 to 9e4fde6 Compare November 21, 2023 16:21
@tmadlener
Copy link
Contributor Author

Implemented the reading of the collections from the command line via the k4FWCore.parseArgs.parser instead of rewriting k4run (#164). This is now ready to be merged.

Copy link
Contributor

@Zehvogel Zehvogel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@tmadlener tmadlener force-pushed the improve-podio-input branch from f8c7325 to da8ba07 Compare December 4, 2023 13:33
@tmadlener tmadlener enabled auto-merge (squash) December 4, 2023 14:01
@tmadlener tmadlener disabled auto-merge December 4, 2023 14:01
@tmadlener tmadlener merged commit d8e5bf4 into key4hep:main Dec 4, 2023
3 of 5 checks passed
@tmadlener tmadlener deleted the improve-podio-input branch December 4, 2023 14:02
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