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

fix usage of obsolete astrometry interfaces in ProcessImageTask

    Details

      Description

      As discussed recently on HipChat (Science Pipelines Standup), there's code in ProocessImageTask that assumes an "astrometer" attribute on a CalibrateTask instance. Since this is just needed to match a new set of sources against the reference catalog here, we should probably be using one of the new matcher objects, either by getting one from CalibrateTask via a documented interface, or by constructing a new one.

        Attachments

          Issue Links

            Activity

            Hide
            jbosch Jim Bosch added a comment -

            A few minor comments on the GitHub PR for pipe_tasks.

            I'm confused as to why ANetAstrometryTask has both a astrometry method and a run method, and both claim to do the same thing with different implementations (and the new AstrometryTask only implements run, so I'm wondering what's still calling astrometry). I'm guessing this is a historical artifact that you'd like to get rid of as much as I would, but I'm curious to hear the explanation.

            Besides that, only comment I have in meas_base is a trivial mistake in the class docs for both AstrometryTask and ANetAstrometryTask, where you've referred to a loadAndRun method instead of loadAndMatch.

            Show
            jbosch Jim Bosch added a comment - A few minor comments on the GitHub PR for pipe_tasks. I'm confused as to why ANetAstrometryTask has both a astrometry method and a run method, and both claim to do the same thing with different implementations (and the new AstrometryTask only implements run , so I'm wondering what's still calling astrometry ). I'm guessing this is a historical artifact that you'd like to get rid of as much as I would, but I'm curious to hear the explanation. Besides that, only comment I have in meas_base is a trivial mistake in the class docs for both AstrometryTask and ANetAstrometryTask , where you've referred to a loadAndRun method instead of loadAndMatch .
            Hide
            rowen Russell Owen added a comment - - edited

            Jim: regarding ANetAstrometryTask.astrometry: that is a private implementation. I'll rename it to _astrometry and update the one place it is used in obs_subaru accordingly. I think it deserves to stay, because it is the core implementation of solving for a WCS; "solve" performs some extra steps to remove the effects of distortion and then reinstate them, which are best left out of the core implementation.

            I found one more place where old interfaces were used: ImageDifferenceTask in a workaround for a bug in the TAN-SIP solver that has since been fixed. I fixed it by removing the workaround.

            Show
            rowen Russell Owen added a comment - - edited Jim: regarding ANetAstrometryTask.astrometry: that is a private implementation. I'll rename it to _astrometry and update the one place it is used in obs_subaru accordingly. I think it deserves to stay, because it is the core implementation of solving for a WCS; "solve" performs some extra steps to remove the effects of distortion and then reinstate them, which are best left out of the core implementation. I found one more place where old interfaces were used: ImageDifferenceTask in a workaround for a bug in the TAN-SIP solver that has since been fixed. I fixed it by removing the workaround.
            Hide
            rowen Russell Owen added a comment - - edited

            Could you please look review my changes to obs_subaru tickets/DM-2939? I updated two config files in order to use meas_astrom AstrometryTask as the default astrometry task and deleted SubaruAstrometryTask (in astrometry.py).

            I tested by running a bit of HSC data through processCcd.py using the default astrometry task (AstrometryTask) and overriding that with the older ANetAstrometryTask and both run.

            Note: I merged meas_astrom and pipe_tasks to master.

            Show
            rowen Russell Owen added a comment - - edited Could you please look review my changes to obs_subaru tickets/ DM-2939 ? I updated two config files in order to use meas_astrom AstrometryTask as the default astrometry task and deleted SubaruAstrometryTask (in astrometry.py). I tested by running a bit of HSC data through processCcd.py using the default astrometry task (AstrometryTask) and overriding that with the older ANetAstrometryTask and both run. Note: I merged meas_astrom and pipe_tasks to master.
            Hide
            jbosch Jim Bosch added a comment -

            Changes in obs_subaru look fine.

            Show
            jbosch Jim Bosch added a comment - Changes in obs_subaru look fine.
            Hide
            rowen Russell Owen added a comment -

            Thank you for the review. I merged obs_subaru to master.

            Show
            rowen Russell Owen added a comment - Thank you for the review. I merged obs_subaru to master.

              People

              • Assignee:
                rowen Russell Owen
                Reporter:
                jbosch Jim Bosch
                Reviewers:
                Jim Bosch
                Watchers:
                Jim Bosch, Lauren MacArthur, Paul Price, Russell Owen
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel