Fix Version/s: None
Team:Data Release Production
Make any changes in the Functor Key capabilities necessary to support getXXXKey, getXXXErrKey, and getXXXFlagKey for the 3 different types of slots.
Change these routines in Source.h.m4 to return FunctorKeys for both version 0 and version 1 tables. Then fixup any compilation breaks on the C++ size which this causes. Remove the version 1 specific accessors.
I am assigning this to Jim to confirm that the first part is done, then he can assign the remainder to Perry either by reassigning, or as a subtask.
- relates to
DM-3213 Get ImageDifferenceTask running again
I think that the only controversial thing in this checkin is that I removed the ability to save or load slots which are defined with non-alike names. This was originally permitted by the constructor which allowed a slot to be defined with 3 (possibly compound) keys for meas, err, and flag. I couldn't find anywhere is the code where this was used and then the slot was permanently saved to a table, so I believe it to be safe.
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.
The FunctorKeys don't quite have everything you need, but I can easily describe what needs to be added so Perry can do it:
In CovarianceMatrixKey, we need to add a bool data member that controls whether the saved values along the matrix diagonal are variances (version 0) or sigmas (version 1). We'll also need to be able to set that in the constructor. With that in place, we should be able to use all of the FunctorKeys to return the version 0 slots.
I'm hoping you can implement this by just storing the version 0 slot keys within FunctorKeys, so we don't have to create them on-the-fly (in the case of CovarianceMatrix, at least, constructing them will be a bit of a pain so it'd be nice to only do it when the slot is defined). But I haven't looked in detail to see if that will be possible.