DM-44029: Deprecate NaiveDipoleCentroid and NaiveDipoleFlux #313
Conversation
| @@ -208,7 +208,9 @@ inline DipoleFluxAlgorithm::DipoleFluxAlgorithm( | |||
| class that knows how to calculate centroids as a simple unweighted first | |||
| * moment of the 3x3 region around the peaks | |||
| */ | |||
| class NaiveDipoleFlux : public DipoleFluxAlgorithm { | |||
| class [[deprecated( | |||
| "This algorithm is deprecated and will be removed after v27.")]] | |||
There was a problem hiding this comment.
Could you include the names of the algorithms in the deprecation warnings?
There was a problem hiding this comment.
That comes "for free" with the compiler warning that is emitted when you try to use the class.
| _posCentroid = meas::base::CentroidResultKey(schema[name+"_pos_centroid"]); | ||
| _negCentroid = meas::base::CentroidResultKey(schema[name+"_neg_centroid"]); | ||
| _avgCentroid = meas::base::CentroidResultKey(schema[name+"_centroid"]); |
There was a problem hiding this comment.
Removing the suffixes seem like it could break other codes, or user scripts. For something that's going to be removed soon, is it necessary? At the very least, it should be on its own separate commit.
There was a problem hiding this comment.
Unfortunately, if I don't change it some existing tests (that use DipoleMeasurementTask, which are only used in tests now and one of the things I'd like to deprecate in the future) will fail because they set ip_diffim_PsfDipoleFlux as the centroid slot, and the centroid slot requires that "name_x"/"name_y" be the centroid fields, not "name_centroid_x".
Nothing in the stack has used these values for a long time, so I think we're ok.
I did move them to a separate commit, though.
| @@ -158,6 +161,9 @@ void wrapDipoleAlgorithms(lsst::cpputils::python::WrapperCollection &wrappers) { | |||
| declarePsfDipoleFlux(wrappers); | |||
| } | |||
|
|
|||
| // Remove these pragmas on DM-44030 | |||
| #pragma GCC diagnostic pop | |||
|
|
|||
There was a problem hiding this comment.
Could you add a comment on what these pragma s are doing?
There was a problem hiding this comment.
They're from the C++ deprecation docs: https://developer.lsst.io/stack/deprecating-interfaces.html#c-deprecation
Change PsfDipoleFlux algorithm to not postfix `_centroid` for positions, so that it behaves properly with the slot system.
parejkoj
force-pushed
the
tickets/DM-44029
branch
from
6d8dfb1
to
3ae023f
Compare
parejkoj
force-pushed
the
tickets/DM-44029
branch
from
3ae023f
to
e94c68f
Compare
No description provided.