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

Wrap tests that depend on both image and table with pybind11

    Details

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

      Description

      The following tasks depend on both afw::image and afw::table. Wrap them.

      1. testApCorrMap.py (enable one skipped test)
      2. testBox.py (add a test for the copy constructor)
      3. testExposureTable.py
      4. testPolygon.py
      5. testTableIO.py
      6. testTableUtils.py
      7. testValidPolygon.py
      8. testVisitInfo.py
      9. testWeather.py

        Attachments

          Issue Links

            Activity

            Hide
            rowen Russell Owen added a comment - - edited


            I added some tests and story points because some of the tests require other classes that have not yet been wrapped (especially {{ExposureTable, which brings in a lot), and which have their own tests.

            Show
            rowen Russell Owen added a comment - - edited I added some tests and story points because some of the tests require other classes that have not yet been wrapped (especially {{ExposureTable, which brings in a lot), and which have their own tests.
            Hide
            rowen Russell Owen added a comment -

            I merged the changes to daf_base after Pim Schellart [X] reviewed the pull request. I made the two changes requested by Pim: adding a bit to the new test and adding the test to master (requested on Slack) using DM-8581.

            Show
            rowen Russell Owen added a comment - I merged the changes to daf_base after Pim Schellart [X] reviewed the pull request. I made the two changes requested by Pim: adding a bit to the new test and adding the test to master (requested on Slack) using DM-8581 .
            Hide
            rowen Russell Owen added a comment -

            Wrap more of afw image and table

            Wrap enough to enable the tests listed above.

            WARNING: CoaddInputs is not properly wrapped: attempting to access
            the "visits" or "ccds" fields will cause a segfault. I will create
            a new ticket to address this issue; meanwhile one test
            of testExposureTable.py is skipped for now to dodge it.

            • Wrap afw::geom::polygon::Polygon
            • Wrap more of Calib
            • Wrap CoaddInputs but see warning above
            • Wrap VisitInfo, including giving the constructor named arguments with defaults;
              this eliminates the need for makeVisitInfo, so reimplement that to raise
              a deprecation warning and then call the VisitInfo constructor
            • Wrap a bit more of Wcs
            • Wrap a lot more of Slots
            • Wrap table::Exposure
            • Wrap a bit more of Schema and Source
            • Modify imageLib.py to import lsst.afw.table.io,
              thus allowing wrappers for image object that can be saved to tables
              to inherit from table::Persistable and support FITS I/O.
            Show
            rowen Russell Owen added a comment - Wrap more of afw image and table Wrap enough to enable the tests listed above. WARNING: CoaddInputs is not properly wrapped: attempting to access the "visits" or "ccds" fields will cause a segfault. I will create a new ticket to address this issue; meanwhile one test of testExposureTable.py is skipped for now to dodge it. Wrap afw::geom::polygon::Polygon Wrap more of Calib Wrap CoaddInputs but see warning above Wrap VisitInfo, including giving the constructor named arguments with defaults; this eliminates the need for makeVisitInfo, so reimplement that to raise a deprecation warning and then call the VisitInfo constructor Wrap a bit more of Wcs Wrap a lot more of Slots Wrap table::Exposure Wrap a bit more of Schema and Source Modify imageLib.py to import lsst.afw.table.io, thus allowing wrappers for image object that can be saved to tables to inherit from table::Persistable and support FITS I/O.
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            Looks fine, minor comments on PR.
            The main question revolves about merging API changes with the pybind11 port. This can bite us in the end, although it is often unavoidable.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - Looks fine, minor comments on PR. The main question revolves about merging API changes with the pybind11 port. This can bite us in the end, although it is often unavoidable.
            Hide
            rowen Russell Owen added a comment -

            Thank you for the helpful review. All changes updated, rebased and merged.

            I moved code that might help diagnose the CoaddInputs problem to a new branch u/rowen/debugCoaddInputs.

            Show
            rowen Russell Owen added a comment - Thank you for the helpful review. All changes updated, rebased and merged. I moved code that might help diagnose the CoaddInputs problem to a new branch u/rowen/debugCoaddInputs .

              People

              • Assignee:
                rowen Russell Owen
                Reporter:
                pschella Pim Schellart [X] (Inactive)
                Reviewers:
                Pim Schellart [X] (Inactive)
                Watchers:
                Krzysztof Findeisen, Pim Schellart [X] (Inactive), Russell Owen
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel