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

meas_astrom still using eups in tests

    Details

    • Type: Improvement
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: meas_astrom, pipe_tasks
    • Labels:
      None
    • Story Points:
      2
    • Team:
      Architecture

      Description

      In DM-2636 we modified the tests to be skipped if EUPS is not available. I've had a closer look and all the ones I have glanced at seem to be easily fixable to run without EUPS. The tests seem to be using EUPS to locate the meas_astrom (effectively asking EUPS for the location of the test file), then a path to the astrometry.net test data within the tests/ directory is located and then EUPS is asked to setup astrometry_net_data using that path. Since the table files are all empty this is the equivalent to simply assigning the ASTROMETRY_NET_DATA_DIR environment variable directly to the path in the tests sub-directory.

      Making this change to one of the tests seems to work so I will change the rest.

        Attachments

          Issue Links

            Activity

            Hide
            price Paul Price added a comment -

            Very nice!

            Show
            price Paul Price added a comment - Very nice!
            Hide
            tjenness Tim Jenness added a comment -

            Now I'm pondering pipe_tasks. Two tests in there could do with the same helper code so I guess I just copy it across from meas_astrom. Seems non-optimal but it also doesn't seem like code that is generic enough to go into utils.

            It also strikes me as odd that one of the pipe_tasks examples actually burrows into the test directory of meas_astrom looking for photocal astrometry.net data and test data files. I may just ignore that example for now and let it rely on eups for a bit longer.

            Show
            tjenness Tim Jenness added a comment - Now I'm pondering pipe_tasks . Two tests in there could do with the same helper code so I guess I just copy it across from meas_astrom . Seems non-optimal but it also doesn't seem like code that is generic enough to go into utils . It also strikes me as odd that one of the pipe_tasks examples actually burrows into the test directory of meas_astrom looking for photocal astrometry.net data and test data files. I may just ignore that example for now and let it rely on eups for a bit longer.
            Hide
            tjenness Tim Jenness added a comment -

            Paul, at risk of winding you up, I've made the same changes to pipe_tasks as I made to meas_astrom. I'm using the same ticket as the fixes are identical.

            My main concern is over the copying of the test helper code from meas_astrom.

            I have removed eups from the example code as well, although that does involve some code duplication as the examples do not have access to the test helper code.

            Show
            tjenness Tim Jenness added a comment - Paul, at risk of winding you up, I've made the same changes to pipe_tasks as I made to meas_astrom . I'm using the same ticket as the fixes are identical. My main concern is over the copying of the test helper code from meas_astrom . I have removed eups from the example code as well, although that does involve some code duplication as the examples do not have access to the test helper code.
            Hide
            price Paul Price added a comment -

            Great!

            I think if you discover any more places where it's necessary to use setupAstrometryNetDataDir then you should put it in utils, as I'm already starting to hold my nose against the duplication.

            Show
            price Paul Price added a comment - Great! I think if you discover any more places where it's necessary to use setupAstrometryNetDataDir then you should put it in utils , as I'm already starting to hold my nose against the duplication.
            Hide
            tjenness Tim Jenness added a comment -

            Merged meas_astrom and pipe_tasks to master. Can't find any other candidates for the helper function.

            Show
            tjenness Tim Jenness added a comment - Merged meas_astrom and pipe_tasks to master. Can't find any other candidates for the helper function.

              People

              • Assignee:
                tjenness Tim Jenness
                Reporter:
                tjenness Tim Jenness
                Reviewers:
                Paul Price
                Watchers:
                Joshua Hoblitt, Paul Price, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel