XMLWordPrintable

#### Details

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

#### Description

Add the basic FunctorKeys mechanism, and enough implementations to support Slots.

#### Activity

Hide
Jim Bosch added a comment -

Ready for review on branch u/jbosch/DM-421 of afw:

 afw:u/jbosch/DM-421 % git diff --stat master...u/jbosch/DM-421  include/lsst/afw/table.h | 2 +  include/lsst/afw/table/BaseRecord.h | 21 ++++  include/lsst/afw/table/FunctorKey.h | 56 +++++++++  include/lsst/afw/table/aggregates.h | 228 +++++++++++++++++++++++++++++++++++  include/lsst/afw/table/fwd.h | 3 +  python/lsst/afw/table/Base.i | 51 +++++++-  python/lsst/afw/table/aggregates.i | 95 +++++++++++++++  python/lsst/afw/table/tableLib.i | 1 +  src/table/aggregates.cc | 214 ++++++++++++++++++++++++++++++++  tests/testFunctorKeys.py | 207 +++++++++++++++++++++++++++++++  10 files changed, 872 insertions(+), 6 deletions(-)

Assigning to Perry because this code may be useful in his implementation of DM-433, so it makes sense for him to look it over to see if that's the case.

Show
Jim Bosch added a comment - Ready for review on branch u/jbosch/ DM-421 of afw: afw:u/jbosch/DM-421 % git diff --stat master...u/jbosch/DM-421 include/lsst/afw/table.h | 2 + include/lsst/afw/table/BaseRecord.h | 21 ++++ include/lsst/afw/table/FunctorKey.h | 56 +++++++++ include/lsst/afw/table/aggregates.h | 228 +++++++++++++++++++++++++++++++++++ include/lsst/afw/table/fwd.h | 3 + python/lsst/afw/table/Base.i | 51 +++++++- python/lsst/afw/table/aggregates.i | 95 +++++++++++++++ python/lsst/afw/table/tableLib.i | 1 + src/table/aggregates.cc | 214 ++++++++++++++++++++++++++++++++ tests/testFunctorKeys.py | 207 +++++++++++++++++++++++++++++++ 10 files changed, 872 insertions(+), 6 deletions(-) Assigning to Perry because this code may be useful in his implementation of DM-433 , so it makes sense for him to look it over to see if that's the case.
Hide
Jim Bosch added a comment -

Moving back to "In Progress"; the plan is now that we'll finish DM-433 first, and then add a bit more on this issue to integrate them.

Show
Jim Bosch added a comment - Moving back to "In Progress"; the plan is now that we'll finish DM-433 first, and then add a bit more on this issue to integrate them.
Hide
Jim Bosch added a comment -

Changed our minds; we do want to get this in before DM-433.

Show
Jim Bosch added a comment - Changed our minds; we do want to get this in before DM-433 .
Hide
Perry Gee added a comment -

Did not test the code (except to run the unit test)

Code looks harmless to any code which is not using functorKeys, so I will kick back any programs after testing them with the slot ticket.

I thought that the unit test had reasonable coverage, but lack of comments and the complexity of the third test made it not useful for someone trying to figure out how to use the code.

I'm not sure why self.assertClose was used. Is the order of the calculation not determined when calculating both operands of the comparisons?

Show
Perry Gee added a comment - Did not test the code (except to run the unit test) Code looks harmless to any code which is not using functorKeys, so I will kick back any programs after testing them with the slot ticket. I thought that the unit test had reasonable coverage, but lack of comments and the complexity of the third test made it not useful for someone trying to figure out how to use the code. I'm not sure why self.assertClose was used. Is the order of the calculation not determined when calculating both operands of the comparisons?
Hide
Jim Bosch added a comment -

I've added comments to the test, including one on assertClose, which I'll repeat here: we use assertClose in part because it supports matrices (and none of the other comparisons do), and also because I'm not sure the square root and squaring of the diagonal elements will round-trip exactly, especially when the squaring in C++ is single-precision and the square root in Python is double precision.

Show
Jim Bosch added a comment - I've added comments to the test, including one on assertClose, which I'll repeat here: we use assertClose in part because it supports matrices (and none of the other comparisons do), and also because I'm not sure the square root and squaring of the diagonal elements will round-trip exactly, especially when the squaring in C++ is single-precision and the square root in Python is double precision.

#### People

Assignee:
Jim Bosch
Reporter:
Jim Bosch
Reviewers:
Perry Gee
Watchers:
Jim Bosch, Perry Gee