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

Modify afw tests to support pytest

    XMLWordPrintable

    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:

                  Jenkins

                  No builds found.