Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-477

Convert makePsfCandidates into its own task

    XMLWordPrintable

    Details

    • Type: RFC
    • Status: Implemented
    • Resolution: Done
    • Component/s: DM
    • Labels:
      None

      Description

      While attempting to clean up the starSelector/sourceSelector API, Chris Morrison [X] and I noticed that most uses of starSelectors were calling selectStars instead of run, meaning they were skipping the fact that starSelctor.run() calls makePsfCandidates after selecting stars. If someone called run() the returned catalog might be different from just calling selectStars() because of the effect of makePsfCandidates() (which is not a catalog action). It would greatly simplify our new API (RFC-198) if we had a separate MakePsfCandidatesTask.

      The one non-unittest (MeasurePsfTask) that we could find that currently usesĀ starSelector.run() would be changed to do:

      stars = starSelector.run(sources)
      psfCandidates = makePsfCandidate.run(stars.starCat)
      

      This should also allow us to deal with DM-5680 and DM-5578, by just using that task in the places where we have that repeated code block. It also fully decouples selecting stars (for whatever purpose) from creating psf candidates, and means our uses of sourceSelectors can be brought in line with RFC-352 by calling run() instead of selectStars()

      Think of this as the logical continuation of RFC-154, which started the process of cleaning up our starSelectors.

      As a short pair coding exercise, Chris Morrison [X] and I already done most of the necessary prep work in DM-14102, if you want to see what the implementation would look like. This is also what triggered the filing of RFC-475.

        Attachments

          Issue Links

            Activity

            Hide
            cmorrison Chris Morrison [X] (Inactive) added a comment -

            Having worked on the code I am in support of this change.

            Show
            cmorrison Chris Morrison [X] (Inactive) added a comment - Having worked on the code I am in support of this change.
            Hide
            rowen Russell Owen added a comment -

            +1 It sounds like a a reasonable improvement. Thanks for doing the research to make sure it will be safe.

            Show
            rowen Russell Owen added a comment - +1 It sounds like a a reasonable improvement. Thanks for doing the research to make sure it will be safe.
            Hide
            Parejkoj John Parejko added a comment -

            Adopting as written.

            Show
            Parejkoj John Parejko added a comment - Adopting as written.
            Hide
            Parejkoj John Parejko added a comment -

            Implementation ticket is DM-14102.

            Show
            Parejkoj John Parejko added a comment - Implementation ticket is DM-14102 .

              People

              Assignee:
              Parejkoj John Parejko
              Reporter:
              Parejkoj John Parejko
              Watchers:
              Chris Morrison [X] (Inactive), John Parejko, Russell Owen
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Planned End:

                  Jenkins

                  No builds found.