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

FrameSet frames not preserved by Transform(frameSet) constructor

    XMLWordPrintable

    Details

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

      Description

      When an lsst.afw.geom.Transform is constructed from an astshim.FrameSet the internal frames in the frame set are lost. I strongly suspect what is happening is that the mapping constructor of Transform is being called, instead of the FrameSet constructor. Reversing the order in the pybind11 wrapper file should fix the problem. A unit test is required.

        Attachments

          Issue Links

          is blocked by

          Bug - A problem which impairs or prevents the functions of the product. DM-9899 FrameSet.permAxes will fail if nIn != nOut

          • Done

            Activity

            Ascending order - Click to sort in descending order
            Hide
            Permalink
            rowen Russell Owen added a comment - - edited

            Transform(FrameSet) was never called in Python, because
            Transform(Mapping) was listed first in the pybind11 wrapper,
            and so was always called.
            Fixing this turned up several bugs in normalizing frames:

            • naive tests in PointEndpoint and SpherePointEndpoint
              were fooled when a FrameSet was passed in as a Frame.
            • SpherePointEndpoint needed a tweak so that permAxes
              was called on the frame argument, rather than a copy
              when the frame was a FrameSet.

            I added tests for all this in tests/test_transform.py. I'm afraid that file is getting rather large, but I'm not sure what to do about it. Perhaps some refactoring could make it smaller.

            I also replaced all usage of self.assertTrue(numpy.allclose(...)) with numpy.testing.assert_allclose(...) as a separate commit.

            Show
            rowen Russell Owen added a comment - - edited Transform(FrameSet) was never called in Python, because Transform(Mapping) was listed first in the pybind11 wrapper, and so was always called. Fixing this turned up several bugs in normalizing frames: naive tests in PointEndpoint and SpherePointEndpoint were fooled when a FrameSet was passed in as a Frame. SpherePointEndpoint needed a tweak so that permAxes was called on the frame argument, rather than a copy when the frame was a FrameSet. I added tests for all this in tests/test_transform.py . I'm afraid that file is getting rather large, but I'm not sure what to do about it. Perhaps some refactoring could make it smaller. I also replaced all usage of self.assertTrue(numpy.allclose(...)) with numpy.testing.assert_allclose(...) as a separate commit.
            Hide
            Permalink
            krzys Krzysztof Findeisen added a comment -

            I requested a few changes on GitHub, but overall looks nice. I especially like the extra testing infrastructure you added to test_transform.py.

            Show
            krzys Krzysztof Findeisen added a comment - I requested a few changes on GitHub, but overall looks nice. I especially like the extra testing infrastructure you added to test_transform.py .
            Hide
            Permalink
            rowen Russell Owen added a comment -

            Thank you for your very helpful review. I feel test_transform.py is much better based on your suggestions and some of the documentation should also be clearer. I have rebased the changes and will merge to master once a Jenkins run passes.

            Show
            rowen Russell Owen added a comment - Thank you for your very helpful review. I feel test_transform.py is much better based on your suggestions and some of the documentation should also be clearer. I have rebased the changes and will merge to master once a Jenkins run passes.

              People

              Assignee:
              rowen Russell Owen
              Reporter:
              rowen Russell Owen
              Reviewers:
              Krzysztof Findeisen
              Watchers:
              Krzysztof Findeisen, Russell Owen
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.