-
Notifications
You must be signed in to change notification settings - Fork 11
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
Fix compiler warnings related to lossy conversion and new name for ROOTWriter #46
Conversation
k4EDM4hep2LcioConv/include/k4EDM4hep2LcioConv/k4EDM4hep2LcioConv.ipp
Outdated
Show resolved
Hide resolved
@@ -100,8 +100,8 @@ namespace LCIO2EDM4hepConv { | |||
lval.setColorFlow(edm4hep::Vector2i(rval->getColorFlow())); | |||
lval.setVertex(edm4hep::Vector3d(rval->getVertex())); | |||
lval.setEndpoint(edm4hep::Vector3d(rval->getEndpoint())); | |||
lval.setMomentum(Vector3fFrom(rval->getMomentum())); | |||
lval.setMomentumAtEndpoint(Vector3fFrom(rval->getMomentumAtEndpoint())); | |||
lval.setMomentum(rval->getMomentum()); |
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.
Can we also remove the Vector3fFrom
now, or is that still in use in other places?
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.
I saw it in use in another place so I didn't remove it
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.
Ah, it looks like there are more overloads. I think this one should be removable now:
k4EDM4hep2LcioConv/k4EDM4hep2LcioConv/include/k4EDM4hep2LcioConv/k4Lcio2EDM4hepConv.h
Line 163 in 697a942
inline edm4hep::Vector3f Vector3fFrom(const double* v) { return edm4hep::Vector3f(v[0], v[1], v[2]); } |
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.
It is actually not due to key4hep/EDM4hep#265
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.
I will merge tomorrow, unless you want to add something here still.
BEGINRELEASENOTES
ROOTWriter
(Remove theFrame
from the default readers and writers AIDASoft/podio#549)ENDRELEASENOTES
They seem to appear only with clang 16 but not with gcc