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

Modify afw tests to support pytest

    Details

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

      Description

      This ticket is for the work of migrating the AFW tests such that they run with the py.test test runner.

        Attachments

          Issue Links

            Activity

            Hide
            tjenness Tim Jenness added a comment -

            Change the product to "afw", check the box to disable the demo.

            Show
            tjenness Tim Jenness added a comment - Change the product to "afw", check the box to disable the demo.
            Hide
            fred3m Fred Moolekamp added a comment -
            Show
            fred3m Fred Moolekamp added a comment - Jenkins build passed ( https://ci.lsst.codes/job/stack-os-matrix/15104/ ).
            Hide
            rhl Robert Lupton added a comment -

            Robert Lupton, I see your point but I think that if a python utility function is created we are still left with the problem of changing the default mask planes in multiple places (if they ever change). At the very least the object that defines the default masked planes should be public, which it currently isn't.

            The "multiple places" being one C++ and one python function? True. So that's an argument for writing the utility function in C++ so it can be called by the Mask ctor. Actually, it is in C++ (setInitMaskBits() in an anon namespace so not part of the API — although that assumes an empty dict and doesn't empty it first).

            Incidentally, I don't like using swig to mess with the API – if you want to extend the API, please extend it in C++. One reason for people being confused by swig is the tendency for some of our wrappers to be full of %extend blocks instead of letting swig do its thing. I hope that this isn't going to complicate the pybind11 switch too much (if it happens)

            Show
            rhl Robert Lupton added a comment - Robert Lupton, I see your point but I think that if a python utility function is created we are still left with the problem of changing the default mask planes in multiple places (if they ever change). At the very least the object that defines the default masked planes should be public, which it currently isn't. The "multiple places" being one C++ and one python function? True. So that's an argument for writing the utility function in C++ so it can be called by the Mask ctor. Actually, it is in C++ (setInitMaskBits() in an anon namespace so not part of the API — although that assumes an empty dict and doesn't empty it first). Incidentally, I don't like using swig to mess with the API – if you want to extend the API, please extend it in C++. One reason for people being confused by swig is the tendency for some of our wrappers to be full of %extend blocks instead of letting swig do its thing. I hope that this isn't going to complicate the pybind11 switch too much (if it happens)
            Hide
            rowen Russell Owen added a comment -

            The changes look good to me. Thank you for being willing to take the time for the cleanup pass.

            Show
            rowen Russell Owen added a comment - The changes look good to me. Thank you for being willing to take the time for the cleanup pass.
            Hide
            fred3m Fred Moolekamp added a comment -

            Sure, thanks for helping with the review. I think this ticket is finally done!

            Show
            fred3m Fred Moolekamp added a comment - Sure, thanks for helping with the review. I think this ticket is finally done!

              People

              • Assignee:
                fred3m Fred Moolekamp
                Reporter:
                tjenness Tim Jenness
                Reviewers:
                Russell Owen, Tim Jenness
                Watchers:
                Fred Moolekamp, John Parejko, Paul Price, Pim Schellart [X] (Inactive), Robert Lupton, Russell Owen, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel