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

Standardize the point of entry for all Tasks

    XMLWordPrintable

    Details

    • Type: RFC
    • Status: Adopted
    • Resolution: Unresolved
    • Component/s: DM
    • Labels:
      None

      Description

      Task​s form the reusable building blocks of algorithmic code. While they are often accessed from the command line, by executing a CmdLineTask, they may also be used directly by the Python programmer from within a script or notebook.

      Across the stack and its documentation, there are a few different conventions for the names given to methods used as points of entry to the Task​ logic:

      • Sometimes, we use a run method which takes a list of explicit data types;
      • Sometimes, we use a run method which takes a Butler dataRef;
      • Sometimes, we provide a runDataRef method which takes a dataRef, unpacks it into its constituent parts, and calls run appropriately;
      • Sometimes, rather than providing run as the primary point of entry to the task, we use some other name which reflects its functionality (e.g. CharacterizeImageTask.characterize, CalibrateTask.calibrate, etc).

      For the convenience of Python programmers, who would like to be able to call Task​s in a uniform way, and to ensure that the arguments to every Task are explicit (rather than an opaque blob), I suggest that we should standardize our approach as follows:

      • Every Task (including CmdLineTask​s) must provide a run method as their primary point of entry. This must take as explicit arguments everything the Task needs to get its job done (ie, not a dataRef).
      • Task​s may provide a runDataRef method which accepts a dataRef as input, extracts the necessary information from that dataRef, and then calls run.

      Note that adopting this RFC will involve not just changing our documentation and updating existing Task​s to follow the new convention, but also changing the logic in pipe_base which calls CmdLineTask.run with a dataRef.

        Attachments

          Issue Links

            Activity

            Hide
            swinbank John Swinbank added a comment -

            Thanks Russell Owen, that's useful.

            So here's my proposal: we document and require that the normal entry point to tasks is run. We will permit exceptions to that, by means of RFC — anybody who is (re-)designing or implementing a task, and who doesn't feel that run is appropriate, can file an RFC to explain why and get an exemption.

            Implementation tickets for this RFC will including fixing up all old tasks to the new standards. As that happens, we'll file RFCs for those tasks (presumably to include LoadReferenceObjects) where we feel an exemption is appropriate.

            Objections?

            Show
            swinbank John Swinbank added a comment - Thanks Russell Owen , that's useful. So here's my proposal: we document and require that the normal entry point to tasks is run . We will permit exceptions to that, by means of RFC — anybody who is (re-)designing or implementing a task, and who doesn't feel that run is appropriate, can file an RFC to explain why and get an exemption. Implementation tickets for this RFC will including fixing up all old tasks to the new standards. As that happens, we'll file RFCs for those tasks (presumably to include LoadReferenceObjects ) where we feel an exemption is appropriate. Objections?
            Hide
            swinbank John Swinbank added a comment -

            I've added two implementation tickets to the RFC and intend to adopt it on that basis unless somebody objects today.

            Show
            swinbank John Swinbank added a comment - I've added two implementation tickets to the RFC and intend to adopt it on that basis unless somebody objects today.
            Hide
            rowen Russell Owen added a comment -

            Let's try that. I'm uneasy about requiring an RFC, in that it sends the message that we don't trust our developers to do the right thing. But it's much better than having no exceptions allowed, and if makes sense to be especially careful until the new rule becomes ingrained.

            Show
            rowen Russell Owen added a comment - Let's try that. I'm uneasy about requiring an RFC, in that it sends the message that we don't trust our developers to do the right thing. But it's much better than having no exceptions allowed, and if makes sense to be especially careful until the new rule becomes ingrained.
            Hide
            swinbank John Swinbank added a comment -

            Thanks Russell Owen, all.

            Show
            swinbank John Swinbank added a comment - Thanks Russell Owen , all.
            Hide
            gpdf Gregory Dubois-Felsmann added a comment -

            This RFC was just mentioned on #dm-middleware-dev in the context of a PR against daf_butler.

            Seeing that it was still in ADOPTED state, I had a look, and the implementation ticket DM-11098 "Audit existing tasks and update them to comply with RFC-352" is still in the To-Do state. This seems like it has surely been superseded by the Gen3 migration - and in addition all the subtasks of DM-11098 are complete.

            Can we close DM-11098 and move this RFC to IMPLEMENTED?

            Show
            gpdf Gregory Dubois-Felsmann added a comment - This RFC was just mentioned on #dm-middleware-dev in the context of a PR against daf_butler . Seeing that it was still in ADOPTED state, I had a look, and the implementation ticket DM-11098 "Audit existing tasks and update them to comply with RFC-352 " is still in the To-Do state. This seems like it has surely been superseded by the Gen3 migration - and in addition all the subtasks of DM-11098 are complete. Can we close DM-11098 and move this RFC to IMPLEMENTED ?

              People

              Assignee:
              Unassigned Unassigned
              Reporter:
              swinbank John Swinbank
              Watchers:
              Andy Salnikov, Gregory Dubois-Felsmann, Hsin-Fang Chiang, Ian Sullivan, John Parejko, Jonathan Sick, Kian-Tat Lim, Meredith Rawls, Nate Lust, Robert Lupton, Russell Owen, Simon Krughoff, Yusra AlSayyad
              Votes:
              0 Vote for this issue
              Watchers:
              13 Start watching this issue

                Dates

                Created:
                Updated:
                Planned End: