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

Rename jointcal.gtransfo

    Details

      Description

      jointcal's Gtransfo object (a mutable 2->2 transform designed for astrometry and optimized for being fit) should have a name that better matches our standards. I created PhotometryTransform, so I suppose an appropriate new name might be AstrometryTransform?

        Attachments

          Issue Links

            Activity

            Hide
            Parejkoj John Parejko added a comment -

            I'll do this as soon as DM-13270 is merged.

            Show
            Parejkoj John Parejko added a comment - I'll do this as soon as DM-13270 is merged.
            Hide
            Parejkoj John Parejko added a comment -

            Paul Price: In the interest of letting no good deed go unpunished, would you be willing to review this massive changeset (that you'd originally requested)? You can choose how to handle the review: I'm not sure it's worth your looking at every line, given that it's roughly +/-1400 lines (more + than - because of reflowing lines that became longer), but I don't know how else to do it.

            I tried to be as thorough as I could be, using a few regexes at the end to catch the remaining transfo* cases, so I don't think I missed any.

            Jenkins run: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/29081/pipeline

            Show
            Parejkoj John Parejko added a comment - Paul Price : In the interest of letting no good deed go unpunished, would you be willing to review this massive changeset (that you'd originally requested)? You can choose how to handle the review: I'm not sure it's worth your looking at every line, given that it's roughly +/-1400 lines (more + than - because of reflowing lines that became longer), but I don't know how else to do it. I tried to be as thorough as I could be, using a few regexes at the end to catch the remaining transfo* cases, so I don't think I missed any. Jenkins run: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/29081/pipeline
            Hide
            price Paul Price added a comment -

            If this is solely a rename and it passes Jenkins, then I'm going to suggest it simply be merged without review.

            Show
            price Paul Price added a comment - If this is solely a rename and it passes Jenkins, then I'm going to suggest it simply be merged without review.
            Hide
            Parejkoj John Parejko added a comment -

            In that case, are you at least happy with the commit messages?

            Show
            Parejkoj John Parejko added a comment - In that case, are you at least happy with the commit messages?
            Hide
            price Paul Price added a comment -

            The first one appears to mix features (renames, fix include guard, cleanup docstrings), but I could believe they are difficult to disentangle from such a wide-ranging change.

            Show
            price Paul Price added a comment - The first one appears to mix features (renames, fix include guard, cleanup docstrings), but I could believe they are difficult to disentangle from such a wide-ranging change.
            Hide
            Parejkoj John Parejko added a comment -

            The include guard fixes were because of the rename (to keep them in sync), as were most of the docstring cleanups.

            Show
            Parejkoj John Parejko added a comment - The include guard fixes were because of the rename (to keep them in sync), as were most of the docstring cleanups.
            Hide
            Parejkoj John Parejko added a comment -

            New names now merged.

            Show
            Parejkoj John Parejko added a comment - New names now merged.

              People

              • Assignee:
                Parejkoj John Parejko
                Reporter:
                Parejkoj John Parejko
                Reviewers:
                Paul Price
                Watchers:
                John Parejko, John Swinbank, Paul Price
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel