Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-427

Name the new WCS class lsst::afw::geom::SkyWcs

    Details

    • Type: RFC
    • Status: Implemented
    • Resolution: Done
    • Component/s: DM
    • Labels:
      None
    • Location:
      here

      Description

      The new composable WCS class that replaces lsst::afw::image::Wcs and TanWcs on DM-10765 is named lsst::afw::geom::SkyWcs. I chose this name to emphasize that this is a 2-dimensional celestial WCS that transforms from pixels to sky coordinates. I feel this is important for two reasons:

      • It gives us an easy way to add specialized WCS classes that perform other useful transforms, such as pixels to wavelength (for spectra).
      • Most competing software (e.g. astropy, AST) treats a WCS as a more flexible object, handling spectra, data cubes, focal plane coordinates and the like.

      I propose to keep the new name.

        Attachments

          Issue Links

            Activity

            Hide
            rhl Robert Lupton added a comment -

            I don't find the abuse of "WCS" to include spectroscopy as relevant here; "World Coordinate System" that got extended to cover "map pixels to lambda". Let's solve the LSST problem.

            I think that this is a bad idea; if it had been mooted before Russel started doing the work I would definitely have voted against it. At this point, it is probably not worth changing back providing that no user code ever sees the new name. In particular, not only do we need to keep Exposure.getWcs() but we need to not add e.g. Exposure.getSkyWcs().

            So I vote, "OK, if no user code knows that this change was made".

            Show
            rhl Robert Lupton added a comment - I don't find the abuse of "WCS" to include spectroscopy as relevant here; "World Coordinate System" that got extended to cover "map pixels to lambda". Let's solve the LSST problem. I think that this is a bad idea; if it had been mooted before Russel started doing the work I would definitely have voted against it. At this point, it is probably not worth changing back providing that no user code ever sees the new name. In particular, not only do we need to keep Exposure.getWcs() but we need to not add e.g. Exposure.getSkyWcs() . So I vote, "OK, if no user code knows that this change was made".
            Hide
            tjenness Tim Jenness added a comment -

            I am confused by this whole discussion. We want strong typing of WCS and WCS is a flexible concept. We deal with that by saying that Exposures must always be 2D sky WCS and so getWcs() returns a SkyWcs. If you have a 2D image of a spectrum your SpectrumExposure.getWcs will return something different. What's the problem with us being explicit about exposures having SkyWcs? It's what is really happening. If you return Wcs then you end up with confusion in your strong typing naming and we deliberately confuse people who understand WCS. Do you envisage AuxTel images being read into Exposure objects? If so how are you going to strongly type getWcs return value?

            Show
            tjenness Tim Jenness added a comment - I am confused by this whole discussion. We want strong typing of WCS and WCS is a flexible concept. We deal with that by saying that Exposures must always be 2D sky WCS and so getWcs() returns a SkyWcs. If you have a 2D image of a spectrum your SpectrumExposure.getWcs will return something different. What's the problem with us being explicit about exposures having SkyWcs? It's what is really happening. If you return Wcs then you end up with confusion in your strong typing naming and we deliberately confuse people who understand WCS. Do you envisage AuxTel images being read into Exposure objects? If so how are you going to strongly type getWcs return value?
            Hide
            rowen Russell Owen added a comment -

            For the record: the class SkyWcs is rarely used directly. However, many packages make a simple WCS for test purposes, and those typically call the factory function makeSkyWcs, a name chosen for the same reason as SkyWcs: to leave room for other kinds of WCS.

            I am not proposing to change the name Exposure.getWcs.

            Show
            rowen Russell Owen added a comment - For the record: the class SkyWcs is rarely used directly. However, many packages make a simple WCS for test purposes, and those typically call the factory function makeSkyWcs , a name chosen for the same reason as SkyWcs : to leave room for other kinds of WCS. I am not proposing to change the name Exposure.getWcs.
            Hide
            rowen Russell Owen added a comment - - edited

            Adopted as follows: the name of the new WCS class is lsst::afw::geom::SkyWcs

            Also the factory functions for creating a simple WCS and a WCS created from FITS metadata are both lsst::afw::geom::makeSkyWcs and calling factory functions is preferred over calling the SkyWcs constructor directly.

            Show
            rowen Russell Owen added a comment - - edited Adopted as follows: the name of the new WCS class is lsst::afw::geom::SkyWcs Also the factory functions for creating a simple WCS and a WCS created from FITS metadata are both lsst::afw::geom::makeSkyWcs and calling factory functions is preferred over calling the SkyWcs constructor directly.
            Hide
            tjenness Tim Jenness added a comment -

            Russell Owen is this RFC now implemented?

            Show
            tjenness Tim Jenness added a comment - Russell Owen is this RFC now implemented?

              People

              • Assignee:
                rowen Russell Owen
                Reporter:
                rowen Russell Owen
                Watchers:
                Jim Bosch, John Parejko, John Swinbank, Kian-Tat Lim, Paul Price, Robert Lupton, Russell Owen, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                8 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Planned End:

                  Summary Panel