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

Replace all use of Coord and subclasses with SpherePoint

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: afw
    • Story Points:
      20
    • Epic Link:
    • Sprint:
      Alert Production F17 - 8, Alert Production F17 - 9, AP S18-2, AP S18-4
    • Team:
      Alert Production

      Description

      Implement RFC-353 by replacing all use of Coord and its subclasses with SpherePoint.

      Things to keep in mind:

      • afw will no longer support coordinate conversions. We do not rely on this now (except for a very small number of cases of use of FK5 J2000 instead if ICRS that is likely a mistake).
      • SpherePoint and Coord have slightly different APIs (for instance SpherePoint is immutable) so some usage will need to change.
      • Coord has some methods that SpherePoint does not. These may have to be added to SpherePoint or implemented as free functions.

      This would also be a good time to eliminate the SpherePoint(double raDecRad[2]) constructor. This was added to support Transform but turned out to be unnecessary.

        Attachments

          Issue Links

          blocks

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-11868 Change Coord objects in ap_association to SpherePoint

          • Won't Fix
          is blocked by

          Improvement - An improvement or enhancement to an existing feature or task. DM-10765 Replace existing WCS classes with SkyWcs

          • Done

          Improvement - An improvement or enhancement to an existing feature or task. DM-11473 Add SpherePoint(long, lat, unit) constructor

          • Done

          Improvement - An improvement or enhancement to an existing feature or task. DM-12924 SpherePoint.offset should work at the poles and for negative offsets

          • Done
          is triggered by

          RFC - An issue whose primary purpose is to advertise an intent and allow stakeholders to comment. RFC-353 Remove all Coord classes and switch to SpherePoint

          • Implemented
          is triggering

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-13902 Build of obs_sdss fails because SpherePoint is missing toIcrs

          • Won't Fix
          relates to

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-9217 Convert some afw enums into class enums

          • To Do

          Improvement - An improvement or enhancement to an existing feature or task. DM-2637 Coord should be abstract

          • Done

          RFC - An issue whose primary purpose is to advertise an intent and allow stakeholders to comment. RFC-367 Add (double, double, units) constructor to SpherePoint and an associated getter

          • Implemented

          Improvement - An improvement or enhancement to an existing feature or task. DM-10999 Make afw.geom.endpoint import all it needs

          • Won't Fix

            Activity

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

            DM-10765 has made the circular import problem for SkyWcs worse. I giving up on having SkyWcs imported into lsst.afw.geom by default; until this ticket is implemented users will have to import SkyWcs and the other symbols defined in skyWcs using something like from lsst.afw.geom.skyWcs import SkyWcs. As part of implementing this ticket, look for such imports and simplify them, and update Python documentation that refers to lsst.afw.geom.skyWcs.SkyWcs.

            Show
            rowen Russell Owen added a comment - DM-10765 has made the circular import problem for SkyWcs worse. I giving up on having SkyWcs imported into lsst.afw.geom by default; until this ticket is implemented users will have to import SkyWcs and the other symbols defined in skyWcs using something like from lsst.afw.geom.skyWcs import SkyWcs . As part of implementing this ticket, look for such imports and simplify them, and update Python documentation that refers to lsst.afw.geom.skyWcs.SkyWcs .
            Hide
            Permalink
            rowen Russell Owen added a comment - - edited

            The changes are described in this community posting: https://community.lsst.org/t/lsst-afw-coord-coord-classes-are-going-away/2769

            Most of the packages I had to change have trivial, obvious changes. afw, naturally, needed a lot of changes. Also obs_lsstSim has one non-obvious change: the camera mapper has special logic in std_raw to handle old raw images which used coordinate conversion capabilities of the Coord classes that are gone. I reworked the code (greatly simplifying it). The results are slightly, but not significantly, different.

            Show
            rowen Russell Owen added a comment - - edited The changes are described in this community posting: https://community.lsst.org/t/lsst-afw-coord-coord-classes-are-going-away/2769 Most of the packages I had to change have trivial, obvious changes. afw, naturally, needed a lot of changes. Also obs_lsstSim has one non-obvious change: the camera mapper has special logic in std_raw to handle old raw images which used coordinate conversion capabilities of the Coord classes that are gone. I reworked the code (greatly simplifying it). The results are slightly, but not significantly, different.
            Hide
            Permalink
            rowen Russell Owen added a comment -

            Meredith Rawls please review the changes to the three ap packages: ap_association, ap_pipe and ap_verify

            Show
            rowen Russell Owen added a comment - Meredith Rawls please review the changes to the three ap packages: ap_association, ap_pipe and ap_verify
            Hide
            Permalink
            rowen Russell Owen added a comment -

            Bob Armstrong heroically reviewed all the packages except pipe_analysis. Since that is not a DM package I am leaving it unmerged for now and merging the rest.

            @yusra I will leave it to folks at Princeton to review pipe_analysis when they have time. I am happy to respond to reviewer comments. The changes for this ticket are small, but I also fixed all the pep8 violations, which was a big change (and a separate commit).

            Show
            rowen Russell Owen added a comment - Bob Armstrong heroically reviewed all the packages except pipe_analysis. Since that is not a DM package I am leaving it unmerged for now and merging the rest. @yusra I will leave it to folks at Princeton to review pipe_analysis when they have time. I am happy to respond to reviewer comments. The changes for this ticket are small, but I also fixed all the pep8 violations, which was a big change (and a separate commit).
            Hide
            Permalink
            lauren Lauren MacArthur added a comment -

            Just to follow up...these changes have now been merged to master on pipe_analysis, in particular:

            https://github.com/lsst-dm/pipe_analysis/commit/233ef51dca766f1b07c62eb613fef31f75253f0e

            https://github.com/lsst-dm/pipe_analysis/commit/d1896a66d2319001a39b2029f700fc56caf3976a

            https://github.com/lsst-dm/pipe_analysis/commit/17aec3d6d826cf7e2d7e41512c5bc92e9c48f229

             Sorry it took so long!

            Show
            lauren Lauren MacArthur added a comment - Just to follow up...these changes have now been merged to master on pipe_analysis , in particular: https://github.com/lsst-dm/pipe_analysis/commit/233ef51dca766f1b07c62eb613fef31f75253f0e https://github.com/lsst-dm/pipe_analysis/commit/d1896a66d2319001a39b2029f700fc56caf3976a https://github.com/lsst-dm/pipe_analysis/commit/17aec3d6d826cf7e2d7e41512c5bc92e9c48f229  Sorry it took so long!

              People

              Assignee:
              rowen Russell Owen
              Reporter:
              rowen Russell Owen
              Reviewers:
              Bob Armstrong, Meredith Rawls
              Watchers:
              Bob Armstrong, Colin Slater, Jim Bosch, John Swinbank, Krzysztof Findeisen, Lauren MacArthur, Russell Owen
              Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.