Recreating some comments from Perry and I that were lost in a failed JIRA transition. Perry is in quotes.
In order to get the meas_algorithms unit tests to pass, the order
of aggregates.i in tableLib.i (in afw) was moved up to in front of
Source.i. I thought the change in order would not affect aggregates.i,
but Jim should review.
afw, Source.h.m4: do we need the addCentroidMeasFields() and
addShapeMeasFields() convenience functions? I suspect that they won't
be used anywhere unless we try to convert meas_algorithms-style
algorithms to use version 1 tables (which I don't want to do; I'd rather
convert these algorithms to the meas_base interface).
afw, Source.h.m4: please revert the whitespace change near
makeFitsWriter (newline should go after this method, not before it).
afw, Source.h.m4: why rename .e.g hasCentroid() to hasCentroidSlot()?
(if both of these are moderately new, I don't care, but if hasCentroid()
has been around a while we shouldn't change it without an RFC). And if
we don't change this, then we need no changes for this issue in
meas_astrom or meas_base.
afw, Source.h.m4: after this change, I thought we'd be able to just
remove the old slot data members (_slotFlux, _slotCentroid, _slotShape),
and then only look at tableVersion in the methods that define the slots.
Does that not work?
ip_diffim, dipoleMeasurement.py: please revert the whitespace change
after docstring of SourceFlagChecker.
The convenience functions are used for version 1 tables. There
are several places in places like meas_astrom, ip_diffim, and ap where
compound fields for Shapes or Centroids are added on the fly. I just
want to make it less verbose to add them.
I changed the name of hasCentroid() because the macro to create the
function did not seem to work without something before the (). I am
willing to go back to hasCentroid() if you can tell me what punctuation
I need. But I don't think it matters since I just added this method,
and I actually like hasCentroidSlot better, as it is more
I guess it would be OK to remove the old _slots. We just can't
reinstate the old macros, since some of the slots return Keys, and some
I did not see any places where the convenience functions were used
on this ticket. Are you sure they aren't all in meas_algorithms-style
algorithms that need to be more fully converted to meas_base
I'm fine with leaving hasCentroidSlot().
If it is okay to remove the old _slots, please do.
The use of the convenience functions is in code which I have
already changed in various places, but pulled out when we decided to end
the Sprint without testing it (for example, it is needed in ip_diffim
and ap, and could be used in other places - e.g., the distortedCentroid
I think these functions will be needed in some form in the end, and I
just want to avoid having to change the code back and forth to meet
It will take me an hour or more to fix the _slots. I hadn't really
realized that we were done needing them. I'm so used to being in this
intermediate state of supporting both versions.