Details
-
Type:
Story
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Labels:
-
Story Points:2
-
Epic Link:
-
Sprint:AP F18-3
-
Team:Alert Production
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
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.
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.
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.
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.
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.