# fix names of meas_base plugins to match new naming standards

XMLWordPrintable

#### Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
1
• Sprint:
Measurement-S14-4
• Team:
Data Release Production

#### Description

Some meas_base plugins still have old-style algorithm names.

#### Activity

Hide
Jim Bosch added a comment -

Russell, could you take a look? Most changes are trivial - just a renaming of some plugin algorithms to match new field name conventions. Only non-trivial changes are the addition of a psuedo-algorithm that's intended to fill the shape slot during forced measurement, by transforming the shape from the reference catalog. I also cleaned up the corresponding pseudo-algorithm for centroids.

All changes are in meas_base branch u/jbosch/DM-1010.

 meas_base:u/jbosch/DM-1010 % git diff --stat master...u/jbosch/DM-1010  python/lsst/meas/base/forcedMeasurement.py | 7 ++-  python/lsst/meas/base/plugins.py | 86 ++++++++++++++++++++++++----  tests/testForced.py | 35 ++++++-----  tests/testGaussianCentroidBasic.py | 2 +-  tests/testNaiveCentroidBasic.py | 2 +-  tests/testNaiveFluxBasic.py | 4 +-  tests/testPixelFlagsBasic.py | 4 +-  tests/testPsfFluxBasic.py | 4 +-  tests/testSdssCentroidBasic.py | 6 +-  tests/testSdssShapeBasic.py | 2 +-  tests/testSincFluxBasic.py | 4 +-  tests/testSingleFrameMeasurement.py | 8 +--  tests/testSkyCoordBasic.py | 4 +-  13 files changed, 118 insertions(+), 50 deletions(-)

Show
Jim Bosch added a comment - Russell, could you take a look? Most changes are trivial - just a renaming of some plugin algorithms to match new field name conventions. Only non-trivial changes are the addition of a psuedo-algorithm that's intended to fill the shape slot during forced measurement, by transforming the shape from the reference catalog. I also cleaned up the corresponding pseudo-algorithm for centroids. All changes are in meas_base branch u/jbosch/ DM-1010 . meas_base:u/jbosch/DM-1010 % git diff --stat master...u/jbosch/DM-1010 python/lsst/meas/base/forcedMeasurement.py | 7 ++- python/lsst/meas/base/plugins.py | 86 ++++++++++++++++++++++++---- tests/testForced.py | 35 ++++++----- tests/testGaussianCentroidBasic.py | 2 +- tests/testNaiveCentroidBasic.py | 2 +- tests/testNaiveFluxBasic.py | 4 +- tests/testPixelFlagsBasic.py | 4 +- tests/testPsfFluxBasic.py | 4 +- tests/testSdssCentroidBasic.py | 6 +- tests/testSdssShapeBasic.py | 2 +- tests/testSincFluxBasic.py | 4 +- tests/testSingleFrameMeasurement.py | 8 +-- tests/testSkyCoordBasic.py | 4 +- 13 files changed, 118 insertions(+), 50 deletions(-)
Hide
Jim Bosch added a comment -

Just added GitHub PR to help with the review (linked from Issue page).

Show
Jim Bosch added a comment - Just added GitHub PR to help with the review (linked from Issue page).
Hide
Russell Owen added a comment -

On the whole this looks just fine to me. The more complete transform in ForcedTransformedShapePlugin is nice. The new names seem fine.

I confess that I did not fully understand the changes to testForced.py (in particular refCat.table.defineCentroid("truth")) but I'm not worried about it.

My only substantive complaint is as follows:

The new process for making keys that apply to multiple fields seems somewhat clumsy and repetitive.

  # Allocate x and y fields, join these into a single FunctorKey for ease-of-use.  xKey = schema.addField(name + "_x", type="D", doc="transformed reference centroid column",  units="pixels")  yKey = schema.addField(name + "_y", type="D", doc="transformed reference centroid row",  units="pixels")  self.centroidKey = lsst.afw.table.Point2DKey(xKey, yKey)

A loop reduces repetition (though one could argue about readability), and that might be enough for Point2, Extent2 and perhaps even Quadrupole:

  keyList = [schema.addField("%s_%s" % (name, axis), type="D",  doc="transformed reference centroid, in %s" % (axis,),  units="pixels") for axis in ("x", "y")]  self.centroidKey = lsst.afw.table.Point2DKey(*keyList)

but it's not all that readable and it's not really enough for more complex common objects such as Box2I. I think some syntactic sugar, such as factory functions that construct keys for Box2, Point2, Extent2 and Quadrupole would be very helpful for making the user's intent clearer. For example:

  self.centroidKey = lsst.afw.table.makePoint2Key(name, type="D", doc="transformed reference centroid",  units="pixels")

In addition to readability and reduced danger of typos, it also enforces the conventions for field name suffixes, though at the expense of a bit of "magic" and non-explicit field names.

Show
Hide
Jim Bosch added a comment -

I found myself unhappy with this level of boilerplate too, and while working on the issue, I added a note to DM-423 (a bit cryptic, looking at it today; will elaborate) to address this issue there. Generally speaking, while we put together syntactic sugar to allocate these fields in a consistent way in C++, we haven't yet made those interfaces easily usable from Python, and we definitely need to do that.

Show
Jim Bosch added a comment - I found myself unhappy with this level of boilerplate too, and while working on the issue, I added a note to DM-423 (a bit cryptic, looking at it today; will elaborate) to address this issue there. Generally speaking, while we put together syntactic sugar to allocate these fields in a consistent way in C++, we haven't yet made those interfaces easily usable from Python, and we definitely need to do that.
Hide
Jim Bosch added a comment -

Merged to master.

Show
Jim Bosch added a comment - Merged to master.

#### People

Assignee:
Jim Bosch
Reporter:
Jim Bosch
Reviewers:
Russell Owen
Watchers:
Jim Bosch, Russell Owen