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

Remove aliased afwGeom geom usage from stack

    XMLWordPrintable

    Details

    • Story Points:
      6
    • Sprint:
      Arch 2019-07-15, Arch 2019-07-22
    • Team:
      Architecture

      Description

      We have deprecated lsst.afw.geom.Point2I and related routines but they are still used all over the place. They need to be removed so that to allow DM-20565 to be unblocked.

        Attachments

          Issue Links

            Activity

            No builds found.
            tjenness Tim Jenness created issue -
            tjenness Tim Jenness made changes -
            Field Original Value New Value
            Link This issue blocks DM-20565 [ DM-20565 ]
            tjenness Tim Jenness made changes -
            Link This issue relates to DM-17566 [ DM-17566 ]
            tjenness Tim Jenness made changes -
            Component/s coadd_utils [ 12600 ]
            Component/s meas_extensions_photometryKron [ 12318 ]
            Component/s meas_extensions_shapeHSM [ 10740 ]
            Component/s obs_base [ 10719 ]
            Component/s obs_test [ 10765 ]
            Component/s shapelet [ 11310 ]
            Component/s afw [ 10714 ]
            Hide
            Parejkoj John Parejko added a comment -

            It looks like your C++ modifications include a clang-format pass? Might be good to do a single clang-format on the src and include directories first, before you modify anything, as not everyone runs it on save, so small C++ often also include other formatting changes because of that.

            I've done these:

            meas_extensions_shapeHSM
            coadd_utils
            obs_base
            obs_test
            shapelet

            Show
            Parejkoj John Parejko added a comment - It looks like your C++ modifications include a clang-format pass? Might be good to do a single clang-format on the src and include directories first, before you modify anything, as not everyone runs it on save, so small C++ often also include other formatting changes because of that. I've done these: meas_extensions_shapeHSM coadd_utils obs_base obs_test shapelet
            Hide
            tjenness Tim Jenness added a comment -

            Thanks. Actually, my editor strips trailing whitespace automatically and I didn't notice it do that. I'll see if I can split the commits up.

            Show
            tjenness Tim Jenness added a comment - Thanks. Actually, my editor strips trailing whitespace automatically and I didn't notice it do that. I'll see if I can split the commits up.
            Hide
            swinbank John Swinbank added a comment -

            Tim, it looks like you're actually doing this work, so I'm assigning the ticket to you!

            Show
            swinbank John Swinbank added a comment - Tim, it looks like you're actually doing this work, so I'm assigning the ticket to you!
            swinbank John Swinbank made changes -
            Assignee Tim Jenness [ tjenness ]
            swinbank John Swinbank made changes -
            Team Architecture [ 10304 ]
            Hide
            tjenness Tim Jenness added a comment -

            I'm trying to stop doing it...

            Show
            tjenness Tim Jenness added a comment - I'm trying to stop doing it...
            Hide
            tjenness Tim Jenness added a comment - - edited

            I merged meas_extensions_psfex.

            Currently ready for review:

            • meas_modelfit
            • obs_cfht
            • meas_extensions_simpleShape
            • meas_extensions_photometryKron

            meas_modelfit needed a bit more work than usual because the tests needed some clean up and one of the test files had been disabled by mistake for 2 years because it was renamed and lost its .py extension.

            Show
            tjenness Tim Jenness added a comment - - edited I merged meas_extensions_psfex. Currently ready for review: meas_modelfit obs_cfht meas_extensions_simpleShape meas_extensions_photometryKron meas_modelfit needed a bit more work than usual because the tests needed some clean up and one of the test files had been disabled by mistake for 2 years because it was renamed and lost its .py extension.
            Hide
            tjenness Tim Jenness added a comment -

            More ready for review:

            • pipe_tasks
            • meas_astrom
            • meas_deblender
            • ip_diffim

            John Parejko would you be able to take a quick look at these? The meas_modelfit is the only one with additional changes.

            Show
            tjenness Tim Jenness added a comment - More ready for review: pipe_tasks meas_astrom meas_deblender ip_diffim John Parejko would you be able to take a quick look at these? The meas_modelfit is the only one with additional changes.
            Hide
            Parejkoj John Parejko added a comment -

            Can you please assign them to someone else? I'm dealing with the monster (DM-20286).

            Show
            Parejkoj John Parejko added a comment - Can you please assign them to someone else? I'm dealing with the monster ( DM-20286 ).
            Hide
            tjenness Tim Jenness added a comment -

            John Parejko Of course. I asked whether you could look at them. "No" is an acceptable answer.

            Show
            tjenness Tim Jenness added a comment - John Parejko Of course. I asked whether you could look at them. "No" is an acceptable answer.
            Hide
            sullivan Ian Sullivan added a comment -

            I reviewed: meas_modelfit, ip_diffim, meas_deblender, and pipe_tasks.

            I can look at the remaining packages if needed.

            Show
            sullivan Ian Sullivan added a comment - I reviewed: meas_modelfit, ip_diffim, meas_deblender, and pipe_tasks. I can look at the remaining packages if needed.
            Hide
            sullivan Ian Sullivan added a comment -

            I believe I have finished reviewing all of the packages. Please let me know if I've missed any.

            Show
            sullivan Ian Sullivan added a comment - I believe I have finished reviewing all of the packages. Please let me know if I've missed any.
            sullivan Ian Sullivan made changes -
            Status To Do [ 10001 ] Reviewed [ 10101 ]
            Hide
            tjenness Tim Jenness added a comment -

            Thank you very much Ian Sullivan. You have reviewed all the open pull requests. There are now only a handful of packages remaining (including jointcal and obs_subaru).

            Show
            tjenness Tim Jenness added a comment - Thank you very much Ian Sullivan . You have reviewed all the open pull requests. There are now only a handful of packages remaining (including jointcal and obs_subaru).
            tjenness Tim Jenness made changes -
            Component/s ip_diffim [ 10743 ]
            Component/s ip_isr [ 10730 ]
            Component/s meas_extensions_astrometryNet [ 14302 ]
            Component/s meas_extensions_convolved [ 13632 ]
            Component/s meas_extensions_simpleShape [ 13401 ]
            Component/s obs_cfht [ 10762 ]
            Component/s obs_subaru [ 10747 ]
            Component/s pipe_tasks [ 10726 ]
            tjenness Tim Jenness made changes -
            Component/s jointcal [ 13411 ]
            tjenness Tim Jenness made changes -
            Component/s meas_astrom [ 10745 ]
            tjenness Tim Jenness made changes -
            Component/s obs_decam [ 12851 ]
            Component/s obs_lsst [ 16504 ]
            Component/s obs_sdss [ 10763 ]
            tjenness Tim Jenness made changes -
            Sprint Arch 2019-07-15 [ 936 ]
            Story Points 6
            Hide
            tjenness Tim Jenness added a comment -

            Simon Krughoff can you please review obs_sdss and obs_lsstSim changes. Mostly trivial although I fixed a couple of ResourceWarnings in obs_sdss.

            Show
            tjenness Tim Jenness added a comment - Simon Krughoff can you please review obs_sdss and obs_lsstSim changes. Mostly trivial although I fixed a couple of ResourceWarnings in obs_sdss.
            tjenness Tim Jenness made changes -
            Reviewers Simon Krughoff [ krughoff ]
            Status Reviewed [ 10101 ] In Review [ 10004 ]
            tjenness Tim Jenness made changes -
            Reviewers Simon Krughoff [ krughoff ] Ian Sullivan, Simon Krughoff [ sullivan, krughoff ]
            tjenness Tim Jenness made changes -
            Reviewers Ian Sullivan, Simon Krughoff [ sullivan, krughoff ] Ian Sullivan, John Parejko, Simon Krughoff [ sullivan, parejkoj, krughoff ]
            tjenness Tim Jenness made changes -
            Link This issue relates to DM-20546 [ DM-20546 ]
            krughoff Simon Krughoff made changes -
            Reviewers Ian Sullivan, John Parejko, Simon Krughoff [ sullivan, parejkoj, krughoff ] Ian Sullivan, John Parejko [ sullivan, parejkoj ]
            Hide
            tjenness Tim Jenness added a comment -

            I'm going to mark this ticket as complete. It's about 98% complete. meas_mosaic has not been touched but that package is imminently disappearing. The remainder are a few example files and the odd bit of support code that don't stop tests failing. We will need to do a new check when we remove the afwGeom wrappers since we currently aren't preventing people adding new code with afwGeom usage in it.

            Show
            tjenness Tim Jenness added a comment - I'm going to mark this ticket as complete. It's about 98% complete. meas_mosaic has not been touched but that package is imminently disappearing. The remainder are a few example files and the odd bit of support code that don't stop tests failing. We will need to do a new check when we remove the afwGeom wrappers since we currently aren't preventing people adding new code with afwGeom usage in it.
            tjenness Tim Jenness made changes -
            Resolution Done [ 10000 ]
            Status In Review [ 10004 ] Done [ 10002 ]
            tjenness Tim Jenness made changes -
            Sprint Arch 2019-07-15 [ 936 ] Arch 2019-07-15, Arch 2019-07-22 [ 936, 937 ]
            lauren Lauren MacArthur made changes -
            Link This issue relates to DM-24705 [ DM-24705 ]

              People

              Assignee:
              tjenness Tim Jenness
              Reporter:
              tjenness Tim Jenness
              Reviewers:
              Ian Sullivan, John Parejko
              Watchers:
              Ian Sullivan, John Parejko, John Swinbank, Simon Krughoff, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.