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

Use the HTM based reference catalogs in tests

    Details

      Description

      In order to move A.net out of meas_astrom to make it a true dependency, we need to replace its use in tests.

        Attachments

          Issue Links

            Activity

            Hide
            wmwood-vasey Michael Wood-Vasey added a comment -

            Very good. Thanks for the explanation. Makes sense.

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - Very good. Thanks for the explanation. Makes sense.
            Hide
            krughoff Simon Krughoff added a comment -

            Russell Owen these changes look fine. My only real question is why you need the test data in two places. pipe_tasks depends on meas_astrom, so it seems fine. In fact an example in pipe_tasks uses the copy of the data in meas_astrom. Other than some other trivial comments, this is fine to merge.

            Show
            krughoff Simon Krughoff added a comment - Russell Owen these changes look fine. My only real question is why you need the test data in two places. pipe_tasks depends on meas_astrom , so it seems fine. In fact an example in pipe_tasks uses the copy of the data in meas_astrom. Other than some other trivial comments, this is fine to merge.
            Hide
            rowen Russell Owen added a comment - - edited

            You raise a good point about duplication of the data (and thanks for catching the bug that the pipe_tasks example uses the data in meas_astrom), and I am not very happy about the duplication. However, my understanding is that we prefer test data in packages to be self-contained, so that modifying test data in one package won't mysteriously break tests in other packages. And indeed both packages had duplicate a.net versions of this data (tests/astrometry_net_data/photocal), though I was able to delete one of those (the other will move to the new a.net wrapper package as part of separating moving the a.net wrapper code into its own package, DM-2186).

            Show
            rowen Russell Owen added a comment - - edited You raise a good point about duplication of the data (and thanks for catching the bug that the pipe_tasks example uses the data in meas_astrom ), and I am not very happy about the duplication. However, my understanding is that we prefer test data in packages to be self-contained, so that modifying test data in one package won't mysteriously break tests in other packages. And indeed both packages had duplicate a.net versions of this data (tests/astrometry_net_data/photocal ), though I was able to delete one of those (the other will move to the new a.net wrapper package as part of separating moving the a.net wrapper code into its own package, DM-2186 ).
            Hide
            rowen Russell Owen added a comment -

            Regarding the example in pipe_tasks using data from meas_astrom: there is actually a reason for this: the image it uses only lives in meas_astrom, so it makes some sense to use the reference catalog in the same package. I suggest we live with it, though I am not thrilled about it. We could copy the image to pipe_tasks but I am reluctant to add to the existing duplication.

            I have posted a thread on the HipChat DM room to ask about whether we can use a single ref cat in meas_astrom instead of duplicating it, and will try to get that resolved before merging this ticket.

            Show
            rowen Russell Owen added a comment - Regarding the example in pipe_tasks using data from meas_astrom : there is actually a reason for this: the image it uses only lives in meas_astrom , so it makes some sense to use the reference catalog in the same package. I suggest we live with it, though I am not thrilled about it. We could copy the image to pipe_tasks but I am reluctant to add to the existing duplication. I have posted a thread on the HipChat DM room to ask about whether we can use a single ref cat in meas_astrom instead of duplicating it, and will try to get that resolved before merging this ticket.
            Hide
            rowen Russell Owen added a comment -

            Kian-Tat Lim said to keep duplicating the data for now. Eventually it would be best to move it into a test data package.

            Show
            rowen Russell Owen added a comment - Kian-Tat Lim said to keep duplicating the data for now. Eventually it would be best to move it into a test data package.

              People

              • Assignee:
                rowen Russell Owen
                Reporter:
                krughoff Simon Krughoff
                Reviewers:
                Simon Krughoff
                Watchers:
                Michael Wood-Vasey, Russell Owen, Simon Krughoff
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: