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

Add RegistryField support to Task.makeSubtask

    Details

    • Story Points:
      2
    • Team:
      Alert Production

      Description

      As part of implementing RFC-183 add support for tasks specified in lsst.pex.config.RegistryField to lsst.pipe.base.Task.makeSubtask

        Attachments

          Issue Links

            Activity

            Hide
            rowen Russell Owen added a comment -

            It turns out that Task.makeSubtask already supported subtasks specified as a pex_config RegistryField. However, the documentation explicitly said otherwise and it was not tested. So I updated the docs, updated the Task unit test and tweaked a variable name in Task.makeTask to clarify the code.

            Show
            rowen Russell Owen added a comment - It turns out that Task.makeSubtask already supported subtasks specified as a pex_config RegistryField . However, the documentation explicitly said otherwise and it was not tested. So I updated the docs, updated the Task unit test and tweaked a variable name in Task.makeTask to clarify the code.
            Hide
            rowen Russell Owen added a comment -

            I updated the documentation in pipe_base and the "how to write a task" documentation in pipe_tasks, as well as expanding one unit test and renaming one variable in pipe_base.

            Show
            rowen Russell Owen added a comment - I updated the documentation in pipe_base and the "how to write a task" documentation in pipe_tasks, as well as expanding one unit test and renaming one variable in pipe_base.
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            Looks fine. Suggested minor changes.

            One overall comment though is that I think the RegistryField syntax as used for this purpose is highly confusing (and ugly). The simple test case already demonstrates this and I expect it to be far worse in real code. Especially now that there are two ways of specifying subtasks rather than just one. But I realise that this decision has already been made in RFC-183. Moreover, given that this was effectively already supported I reckon it is better to have it tested and documented

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - Looks fine. Suggested minor changes. One overall comment though is that I think the RegistryField syntax as used for this purpose is highly confusing (and ugly). The simple test case already demonstrates this and I expect it to be far worse in real code. Especially now that there are two ways of specifying subtasks rather than just one. But I realise that this decision has already been made in RFC-183 . Moreover, given that this was effectively already supported I reckon it is better to have it tested and documented
            Hide
            rowen Russell Owen added a comment -

            Thank you for the quick and thorough review, including excellent suggestions for improving the clarity of the documentation.

            Show
            rowen Russell Owen added a comment - Thank you for the quick and thorough review, including excellent suggestions for improving the clarity of the documentation.

              People

              • Assignee:
                rowen Russell Owen
                Reporter:
                rowen Russell Owen
                Reviewers:
                Pim Schellart [X] (Inactive)
                Watchers:
                Pim Schellart [X] (Inactive), Russell Owen
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel