Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-421

add basic FunctorKeys

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: afw
    • Labels:

      Description

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

        Attachments

          Activity

          Hide
          jbosch 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
          jbosch 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
          jbosch 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
          jbosch 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
          jbosch Jim Bosch added a comment -

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

          Show
          jbosch Jim Bosch added a comment - Changed our minds; we do want to get this in before DM-433 .
          Hide
          pgee 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
          pgee 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
          jbosch 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
          jbosch 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:
            jbosch Jim Bosch
            Reporter:
            jbosch Jim Bosch
            Reviewers:
            Perry Gee
            Watchers:
            Jim Bosch, Perry Gee
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:

                Jenkins

                No builds found.