Differences on top of u/pgee/DM-1466a as shown below.
While your at it, think about the contents of baseLib.i after we do this merge.

git diff --stat 734b661bab09566e3a68d13643cf6ed9876dc439
include/lsst/meas/base/ApertureFlux.h | 53 ++++++++++++++++++++++++++++-------------------------
include/lsst/meas/base/CircularApertureFlux.h | 56 ++++++++++++++++++++++++++++----------------------------
python/lsst/meas/base/ApertureFlux.i | 10 +---------
python/lsst/meas/base/plugins.py | 54 +-----------------------------------------------------
python/lsst/meas/base/pluginsLib.i | 3 +++
src/ApertureFlux.cc | 38 ++++++++++++++++++++++++++++++++++----
src/CircularApertureFlux.cc | 37 +++++++++++++++++++++++++++++--------
7 files changed, 124 insertions, 127 deletions

This change is now on u/p[gee/DM-1467b.

I messed the git history up when I merged to changes from DM-1466a, so I needed a new branch.

Starting review now; will post more shortly. For now, I have a quick pair of comments on requesting reviews:

• Please use the git diff syntax that shows both the starting and ending points for the comparison, so it's very clear what the comparison is. In this case, I believe that would be git diff --stat u/pgee/DM-1466a...u/pgee/DM-1467b (you can of course also use SHA1s instead of branch names).
• Please paste the comments inside of a JIRA "{code:hide-linenum}...{code}" block to preserve the fixed-width formatting and make the diff more readable.

Review complete. I have a number of comments, most of which are quite minor.

• Doxygen comment intended for ApertureFluxAlgorithm is now right above ApertureFluxResult instead. When you reorder those, also please make sure we have at least one blank lines between classes (I think the standards permit either one or two, and we should just be consistent within one file, I think).

• What was your motivation for making ApertureFluxAlgorithm's _ctrl data member non-const? Making it const was originally intended to prevent a compiler-generated assignment operator from being created, which (since ApertureFluxAlgorithm is an abstract base class) could have caused problems if it was allowed.

• I think it might make sense to move the FlagHandler data member, the initialization of the flags, and adding the radii to the metadata to ApertureFluxAlgorithm; I think any future subclasses of ApertureFluxAlgorithm will want to use the same logic here that CircularApertureFluxAlgorithm does. This isn't a terribly big deal if there's some reason it doesn't work as well there that I've missed; it is quite possible that we'd need to revisit this anyway once we do add another subclass of ApertureFluxAlgorithm.

• Line 47 of CircularApertureFlux.h is longer than the 110-column limit, and hence needs to be split into two or more lines.

• Unnecessary whitespace after "SimpleAlgorithm" in the declaration of CircularFluxAlgorithm's base classes.

• Should add a blank line after declaration of CircularFluxAlgorithm::fail and before declaration of its data members.

• This is a problem in the code I wrote that you started with, but I think the #include "lsst/meas/base/SincCoeffs.h" is unnecessary in CircularFluxAlgorithm.h, and should be removed.

• Another one that's my fault, not yours, but the capitalization of line 38 of ApertureFlux.i needs to be fixed ("computesincFlux"->"computeSincFlux").

As for the future state of baseLib.i, I think we should be able to shrink it dramatically on DM-1468 by removing all the %instantiate and %wrap macros, as well as the %includes of the old utility classes. Was there some other aspect of it you wanted me to consider?

No reason why the _ctrl is not const. I just copied it wrong.

I think that the _centroidExtractor and the _flagHandler should be on the CircularApertureFluxAlgorithm, since to me it goes logically with the SimpleAlgorithm. And ApertureFluxAlgorithm is not a SimpleAlgorithm. But if you are confident that all ApertureFluxes are going to follow this pattern I can move it.

I put the SincCoeff initialization in CircularApertureFlux because that is where you had it. I think it belongs in ApertureFlux.

The initialization of FlagKeys does not belong in CircularApertureFlux. My mistake. The code is a duplication. I put it there by accident when I was removing the FlagHandler array I had put in to CircularApertureFlux. (your recommendation that we not try to do this now).

Ah, I see I had the SincCoeffs right the first time - they do go in ApertureFlux, not CircularApertureFlux, because we explicitly initialize SincCoeffs only for circular apertures.

I should have noticed this earlier too, but I think ApertureFlux could actually inherit from SimpleAlgorithm itself, and implement fail() and deal with the flags too.

Sorry about the confusion.