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

Move makePsfCandidates into its own task

    XMLWordPrintable

    Details

      Description

      To facilitate cleaning up the StarSelector API, we want to remove the makePsfCandidates step from starSelector.run(), and turn it into its own Task. As far as we can tell, only one existing Task (pipe_tasks.measurePsf) uses that feature of StarSelector, so this should be easy to manage.

      John Swinbank suggested a future RFC for merging this: we'll try a quick implementation first, and file a short RFC if it seems disruptive.

        Attachments

          Issue Links

          is duplicated by

          Improvement - An improvement or enhancement to an existing feature or task. DM-5578 making PSF candidates should be simpler

          • Invalid
          is triggered by

          RFC - An issue whose primary purpose is to advertise an intent and allow stakeholders to comment. RFC-477 Convert makePsfCandidates into its own task

          • Implemented
          relates to

          RFC - An issue whose primary purpose is to advertise an intent and allow stakeholders to comment. RFC-475 Remove secondMomentStarSelector

          • Implemented

            Activity

            Ascending order - Click to sort in descending order
            Hide
            Permalink
            Parejkoj John Parejko added a comment - - edited

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

            Show
            Parejkoj John Parejko added a comment - - edited Jenkins run: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/27883/pipeline
            Hide
            Permalink
            Parejkoj John Parejko added a comment -

            Meredith Rawls: can you please review this ticket? It's of "medium" size.

            I strongly suggest reviewing each PR per-commit, since one of the commits on each PR touches a whole bunch of files (the removal of python2 future imports) and is completely separate from the functional change commit.

            Show
            Parejkoj John Parejko added a comment - Meredith Rawls : can you please review this ticket? It's of "medium" size. I strongly suggest reviewing each PR per-commit, since one of the commits on each PR touches a whole bunch of files (the removal of python2 future imports) and is completely separate from the functional change commit.
            Hide
            Permalink
            mrawls Meredith Rawls added a comment -

            The changes accomplish the stated goal and look good to me. I left a few comments over on GitHub, but unless those raise any deeper issues, I think you can merge it. Please ensure you run a linter for the py2 cleanups especially.

            Show
            mrawls Meredith Rawls added a comment - The changes accomplish the stated goal and look good to me. I left a few comments over on GitHub, but unless those raise any deeper issues, I think you can merge it. Please ensure you run a linter for the py2 cleanups especially.
            Hide
            Permalink
            Parejkoj John Parejko added a comment -

            New Jenkins run, since I added a test: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/27905/pipeline

            Show
            Parejkoj John Parejko added a comment - New Jenkins run, since I added a test: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/27905/pipeline
            Hide
            Permalink
            Parejkoj John Parejko added a comment -

            Thank you for the quick review (and re-review after I added the test).

             

            Merged and done. RFC-477 is now implemented!

            Show
            Parejkoj John Parejko added a comment - Thank you for the quick review (and re-review after I added the test).   Merged and done. RFC-477 is now implemented!

              People

              Assignee:
              cmorrison Chris Morrison [X] (Inactive)
              Reporter:
              Parejkoj John Parejko
              Reviewers:
              Meredith Rawls
              Watchers:
              John Parejko, John Swinbank, Meredith Rawls
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.