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

DM-35265: Reduce usage of MeasurementError #213

Merged
merged 6 commits into from Jun 23, 2022
Merged

DM-35265: Reduce usage of MeasurementError #213

merged 6 commits into from Jun 23, 2022

Conversation

timj
Copy link
Member

@timj timj commented Jun 17, 2022

No description provided.

timj added 3 commits June 17, 2022 13:23
It no longer raises MeasurementError so there is no longer
a log message to capture.
We no longer use lsst.log so there is no need to capture
log output to files and read the files back again.
@timj timj changed the title DM-35265: Reduce usage of MeasurementError DM-35265: Reduce usage of MeasurementError in SdssCentroid Jun 17, 2022
timj added 2 commits June 17, 2022 14:05
This makes the code a bit simpler and puts all the flag handling
directly in measure(). It also removes a try/except.
@timj timj changed the title DM-35265: Reduce usage of MeasurementError in SdssCentroid DM-35265: Reduce usage of MeasurementError Jun 17, 2022
src/PsfFlux.cc Show resolved Hide resolved
Copy link
Member

@TallJimbo TallJimbo left a comment

Choose a reason for hiding this comment

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

Looks good! Only comment is a suggestion for how to get rid of the remaining try/catch on LengthError.

@@ -381,8 +367,7 @@ std::pair<MaskedImageT, double> smoothAndBinImage(std::shared_ptr<afw::detection
try {
subImage.reset(new MaskedImageT(mimage, bbox, afw::image::LOCAL));
} catch (pex::exceptions::LengthError &err) {
throw LSST_EXCEPT(MeasurementError, SdssCentroidAlgorithm::EDGE.doc,
SdssCentroidAlgorithm::EDGE.number);
return std::make_tuple(mimage, 0, SdssCentroidAlgorithm::EDGE.number);
Copy link
Member

Choose a reason for hiding this comment

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

If you want to remove this try/catch as well, I think the replacement is:

if (!mimage.getBBox(afw::image::LOCAL).contains(bbox)) {
    return std::make_tuple(mimage, 0, SdssCentroidAlgorithm::EDGE.number);
}
subImage.reset(new MaskedImageT(mimage, bbox, afw::image::LOCAL));

@timj timj marked this pull request as ready for review June 23, 2022 05:06
@timj timj merged commit be2ba20 into main Jun 23, 2022
@timj timj deleted the tickets/DM-35265 branch June 23, 2022 05:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants