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

setLepton information #18

Merged
merged 5 commits into from
Jul 22, 2022
Merged

setLepton information #18

merged 5 commits into from
Jul 22, 2022

Conversation

saswatinandan
Copy link
Collaborator

No description provided.

@saswatinandan saswatinandan marked this pull request as ready for review July 21, 2022 08:30
#include <assert.h> // assert()

#include <TMath.h> // std::fabs()
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this line is necessary.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is for std::fabs, if it is not added, there is compilation error

Copy link
Member

Choose a reason for hiding this comment

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

In that case prefer STL, ie include <cmath>, because that's where std::fabs() actually comes from.

Comment on lines 4 to 5
#include "TallinnNtupleProducer/EvtWeightTools/interface/Data_to_MC_CorrectionInterface_trigger_Base.h"
// Data_to_MC_CorrectionInterface_trigger_Base
Copy link
Member

Choose a reason for hiding this comment

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

Not needed (already included by Data_to_MC_CorrectionInterface_1l_2tau_trigger.h)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed

Comment on lines 4 to 5
#include "TallinnNtupleProducer/EvtWeightTools/interface/lutAuxFunctions.h" // lutWrapperBase, vLutWrapperBase
#include <iostream>
Copy link
Member

Choose a reason for hiding this comment

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

Not needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed

Comment on lines 11 to 12
#include <TMath.h>
// std::fabs()
Copy link
Member

Choose a reason for hiding this comment

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

std::fabs() is available in the STL (<cmath>), which should already be included by the other headers, so not needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

#include "TallinnNtupleProducer/CommonTools/interface/Era.h" // Era, get_era()
#include "TallinnNtupleProducer/Objects/interface/RecoHadTau.h" // RecoHadTau
#include "TallinnNtupleProducer/Objects/interface/RecoLepton.h" // RecoLepton
#include "TallinnNtupleProducer/CommonTools/interface/hadTauDefinitions.h"
Copy link
Member

Choose a reason for hiding this comment

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

If this is included for a reason, add a comment.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I meant to highlight the 8th line here.

Copy link
Member

@ktht ktht left a comment

Choose a reason for hiding this comment

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

Recurring theme is that there are unnecessary or undocumented header inclusions, nothing major.

@ktht ktht merged commit 40577a3 into main Jul 22, 2022
@ktht ktht mentioned this pull request Jul 22, 2022
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.

None yet

2 participants