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

Move astrometry_net wrapper code from meas_astrom to a new package

    Details

    • Story Points:
      2
    • Sprint:
      Science Pipelines DM-S15-1, Alert Production S17 - 4
    • Team:
      Alert Production

      Description

      Remove all astrometry.net wrapper code from meas_astrom and put it in a new package named meas_extensions_astrometryNet.

        Attachments

          Issue Links

            Activity

            Hide
            rowen Russell Owen added a comment - - edited

            I cloned meas_astrom to meas_extensions_astrometryNet, made a new origin for it (in order to retain history for all files), then pruned both the old and new packages. If I did this correctly then no library file or test should be duplicated in the two packages.

            I then updated the code in the new package to use the new naming, and all existing code I found that uses astrometry.net code (which includes some code in pipe_tasks and obs_monocam, both updated). I filed DM-10242 to remove astrometry.net usage from pipe_tasks.

            I also took a cleanup pass for Doxygen errors and pep8-related issues in meas_astrom and meas_extensions_astrometryNet.

            Show
            rowen Russell Owen added a comment - - edited I cloned meas_astrom to meas_extensions_astrometryNet , made a new origin for it (in order to retain history for all files), then pruned both the old and new packages. If I did this correctly then no library file or test should be duplicated in the two packages. I then updated the code in the new package to use the new naming, and all existing code I found that uses astrometry.net code (which includes some code in pipe_tasks and obs_monocam , both updated). I filed DM-10242 to remove astrometry.net usage from pipe_tasks . I also took a cleanup pass for Doxygen errors and pep8-related issues in meas_astrom and meas_extensions_astrometryNet .
            Hide
            rowen Russell Owen added a comment -

            Please ignore the lsstsw branch; it has already been merged

            Show
            rowen Russell Owen added a comment - Please ignore the lsstsw branch; it has already been merged
            Hide
            Parejkoj John Parejko added a comment -

            Only real question was why the PR for meas_extensions_astrometryNet was +115 −18,451 : did something get messed up creating the new product? It'd be nice if it had a totally clean history at the start of its life.

            Show
            Parejkoj John Parejko added a comment - Only real question was why the PR for meas_extensions_astrometryNet was +115 −18,451 : did something get messed up creating the new product? It'd be nice if it had a totally clean history at the start of its life.
            Hide
            rowen Russell Owen added a comment - - edited

            The new package meas_extensions_astrometryNet is really the old package meas_astrom renamed, so as to preserve the history for the a.net-related files that remain. So I don't think you should be worried. However Tim Jenness or others have an opinion as to a better way to do this, and an opinion about whether it's worth trying to fix it if something else would be better.

            Show
            rowen Russell Owen added a comment - - edited The new package meas_extensions_astrometryNet is really the old package meas_astrom renamed, so as to preserve the history for the a.net-related files that remain. So I don't think you should be worried. However Tim Jenness or others have an opinion as to a better way to do this, and an opinion about whether it's worth trying to fix it if something else would be better.
            Hide
            tjenness Tim Jenness added a comment -

            Since meas_astrom still exists and meas_extensions_astrometryNet is an entirely new package that happens to inherit all the history of meas_astrom, I don't think I see a problem.

            Show
            tjenness Tim Jenness added a comment - Since meas_astrom still exists and meas_extensions_astrometryNet is an entirely new package that happens to inherit all the history of meas_astrom , I don't think I see a problem.
            Hide
            rowen Russell Owen added a comment -
            Show
            rowen Russell Owen added a comment - Merged to master and reported to the community as https://community.lsst.org/t/astrometry-net-code-moved-to-meas-extensions-astrometrynet/1825

              People

              • Assignee:
                rowen Russell Owen
                Reporter:
                rowen Russell Owen
                Reviewers:
                John Parejko
                Watchers:
                Dominique Boutigny, John Parejko, Russell Owen, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel