Skip to content

Conversation

@jgalan
Copy link
Member

@jgalan jgalan commented Dec 7, 2022

jgalan Ok: 36

The TRestAnalysisTree, together with the TRestMetadata class are one of the most visited pages on our API documentation pages. Thus, I took a bit of time to review.

Still, it might require your attention to improve the description of the class.

@jgalan jgalan requested review from juanangp, lobis and nkx111 and removed request for lobis and nkx111 December 7, 2022 09:44
/// variables by invoking the `SetObservableValue()` method and then
/// TRestAnalysisTree::Fill() to generate a new entry inside the tree.
/// The code will be simplified while sacrificing a little performance. We can use
/// temporary variable to set observable value directly. We can focus on the analysis
Copy link
Member Author

@jgalan jgalan Dec 7, 2022

Choose a reason for hiding this comment

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

I do not catch the meaning of these 2 last sentences.

In one hand what we do is to encapsulate the use of the tree through the method SetObservableValue so that the user does not need to be playing around with direct variable generation and branch address creation. Perhaps is something like that we should say here?

Copy link
Member

Choose a reason for hiding this comment

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

what we do is to encapsulate the use of the tree through the method SetObservableValue so that the user does not need to be playing around with direct variable generation and branch address creation

Exactly. That shall be one of the selling point of TRestAnalysisTree. We can clarify it in the text.

@jgalan jgalan requested a review from a team December 9, 2022 12:29
@jgalan
Copy link
Member Author

jgalan commented Mar 28, 2023

Please, approve!

@jgalan jgalan merged commit bb7cdf6 into master Mar 30, 2023
@jgalan jgalan deleted the jgalan_tree_documentation branch March 30, 2023 06:47
jgalan added a commit that referenced this pull request May 3, 2023
…umentation"

This reverts commit bb7cdf6, reversing
changes made to 133f2b7.
jgalan added a commit that referenced this pull request May 3, 2023
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.

4 participants