Fix Version/s: None
Sprint:Science Pipelines DM-W15-4
Team:Data Release Production
The last change to ApertureFlux to make it work with the new C++ design changed ApertureFluxAlgorithm into an instantiatable class. However, I have now figured out how to make this work with SWIG while still allowing measure and fail to be defined by default at the ApertureFlux level.. So this issue is to put things back in order.
My only comments are aesthetic:
- ApertureFlux.h:87: Revert whitespace change (there should be a space before curly bracket open).
- CircularApertureFlux.h:60: If you want to put the close-paren and "const" on a separate line, it should be indented to match the open paren, or the arguments should be on a separate line from the open paren. But since it fits on one line, I'd recommend just putting it on the same line as the rest of the declaration. (See https://confluence.lsstcorp.org/pages/viewpage.action?pageId=16908737, section 6-6).
- CircularApertureFlux.cc:60: This line is too long, so it can't go on one line. I'd recommend using the second form in the standards document linked above here, as that's what the rest of the functions in this file use.
Please amend/rebase/squash these changes into your original commit before merging.
I decided that I did not like this solution, so I moved fail() back to ApertureFlux, and found the right combination of declarations to make measure a pure virtual method and fail a virture defined withing ApertureFlux.cc
Yup, I like the new solution better too. But you've now got the "private:" statement in CircularApertureFlux.h between the measure method and its documentation. Other than that, good to merge.
I think the version under u/pgee/
DM-1659is now correct. The error I made was to try to define fail() withing the base class ApertureFlux and measure() within the derived class CircularApertureFlux. That resulted in an undefined dynamic call from SWIG, for reasons I don't understand. The checked in version has both methods defined in the derived class.
include/lsst/meas/base/ApertureFlux.h | 6 +++---
include/lsst/meas/base/CircularApertureFlux.h | 8 +++++++-
python/lsst/meas/base/ApertureFlux.i | 2 +-
src/ApertureFlux.cc | 10 ----------
src/CircularApertureFlux.cc | 4 ++++
5 files changed, 15 insertions, 15 deletions