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

Rename invert() and getInverse() to inverted()

    Details

      Description

      Implement RFC-500 by renaming the invert method of geom AffineTransform, geom LinearTransform and afw GridTransform and the getInverse method of afw Transform and astshim Mapping to inverted.

      Also rename the simplify method of astshim Mapping to simplifed for the same reason. Note that this is little used outside of astshim.

        Attachments

          Issue Links

            Activity

            Hide
            rowen Russell Owen added a comment -

            Retain the old names for backwards compatibility in all but astshim (because our use of astshim is fairly localized) and mark the old methods as deprecated. File a ticket to remove the old names. But if time permits, do update our current code in the DM stack to use the new names. That should go quickly.

            Show
            rowen Russell Owen added a comment - Retain the old names for backwards compatibility in all but astshim (because our use of astshim is fairly localized) and mark the old methods as deprecated. File a ticket to remove the old names. But if time permits, do update our current code in the DM stack to use the new names. That should go quickly.
            Hide
            rowen Russell Owen added a comment -

            The main work is in the following packages:

            • astshim: rename getInverse to inverted and simplify to simplified.
            • afw: add inverted to Transform and GridTransform as the preferred alternative to getInverse for Transform and invert for GridTransform.
            • geom: add inverted to LinearTransform and AffineTransform as the preferred alternative to invert.
            • jointcal: rename invert to inverted and update existing usage of classes from the packages above (if there is any; by doing the rename I handled that).

            For the other packages I just modified them to use inverted instead of the older name. I think in all cases the change was optional (because astshim is used so rarely) but no guarantees.

            I did not bother with backward compatibility for astshim and jointcal because the former is only used by afw (I think) and the latter is not used by any other package.

            Show
            rowen Russell Owen added a comment - The main work is in the following packages: astshim: rename getInverse to inverted and simplify to simplified . afw: add inverted to Transform and GridTransform as the preferred alternative to getInverse for Transform and invert for GridTransform . geom: add inverted to LinearTransform and AffineTransform as the preferred alternative to invert . jointcal: rename invert to inverted and update existing usage of classes from the packages above (if there is any; by doing the rename I handled that). For the other packages I just modified them to use inverted instead of the older name. I think in all cases the change was optional (because astshim is used so rarely) but no guarantees. I did not bother with backward compatibility for astshim and jointcal because the former is only used by afw (I think) and the latter is not used by any other package.
            Hide
            swinbank John Swinbank added a comment -

            Thanks; looks fine.

            I created a PR on afw because I don't think there was one.

            There's a minor nitpick on the on the astshim PR. In addition, I wonder about this comment:

            Each new CmpMap is simplified using `simplify` before being stored in the FrameSet.

            Without digging around in astAddFrame more than I want to, it's not obvious whether that should now be “using `simplified`”, or if this is referring to some function in AST itself (but that seems to be ast_simplify, not just simplify). (To be honest, after a couple of minutes of poking about, I couldn't find where this simplification is happening at all, so I gave up...)

            I'd like to see a ticket for removal of the deprecated functions after some future release.

            Other than that, good to go.

            Show
            swinbank John Swinbank added a comment - Thanks; looks fine. I created a PR on afw because I don't think there was one. There's a minor nitpick on the on the astshim PR. In addition, I wonder about this comment: Each new CmpMap is simplified using `simplify` before being stored in the FrameSet. Without digging around in astAddFrame more than I want to, it's not obvious whether that should now be “using `simplified`”, or if this is referring to some function in AST itself (but that seems to be ast_simplify , not just simplify ). (To be honest, after a couple of minutes of poking about, I couldn't find where this simplification is happening at all, so I gave up...) I'd like to see a ticket for removal of the deprecated functions after some future release. Other than that, good to go.
            Hide
            rowen Russell Owen added a comment -

            Thank you very much. You were right that "using simplify" should be changed to "using simplified" in astshim FrameSet.h. However, that sounds weird so I deleted the "using simplify" instead. I also updated the other comment you noted in the astshim pull request as per a note on github.

            Show
            rowen Russell Owen added a comment - Thank you very much. You were right that "using simplify" should be changed to "using simplified" in astshim FrameSet.h . However, that sounds weird so I deleted the "using simplify" instead. I also updated the other comment you noted in the astshim pull request as per a note on github.
            Hide
            rowen Russell Owen added a comment - - edited

            I filed DM-15417 to remove the deprecated methods after the next major release and issued community post https://community.lsst.org/t/inverted-is-now-the-standard-method-name-to-get-an-inverse-transform/3122 to describe the changes.

            Show
            rowen Russell Owen added a comment - - edited I filed DM-15417 to remove the deprecated methods after the next major release and issued community post https://community.lsst.org/t/inverted-is-now-the-standard-method-name-to-get-an-inverse-transform/3122 to describe the changes.

              People

              • Assignee:
                rowen Russell Owen
                Reporter:
                rowen Russell Owen
                Reviewers:
                John Swinbank
                Watchers:
                John Swinbank, Russell Owen
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel