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

Add `pipe_tasks` as optional dependency to `validate_drp`

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: Validation
    • Labels:
      None

      Attachments

        Activity

        Hide
        wmwood-vasey Michael Wood-Vasey added a comment -

        Very quick review. of adding pipe_tasks as setupOptional and updated README.

        Changes are here:

        https://github.com/lsst/validate_drp/compare/tickets/DM-5393

        Show
        wmwood-vasey Michael Wood-Vasey added a comment - Very quick review. of adding pipe_tasks as setupOptional and updated README. Changes are here: https://github.com/lsst/validate_drp/compare/tickets/DM-5393
        Hide
        rowen Russell Owen added a comment -

        Thank you for adding pipe_tasks as an optional dependency. Now that I see it I am a bit uneasy about the duplication between the ReadMe file and the ups table file. I naively expected that you would add pipe_tasks to the table file but not the ReadMe. Yet the ReadMe has always had redundancies, since it asks the user to setup the desired obs_ package and validation_data_ package. Furthermore, it makes a great training tool for how to run processCcd.py and other tasks. So asking the user to explicitly set up the pipe_tasks in addition to the correct obs_ package and, in this case, validation_data_ package is clearly a good idea. So I like the new ReadMe.

        However, I'm nervous about the existing and new redundancy between the ReadMe and the eups table file. I lean towards removing obs_ and validation_data_ from the ups table file. I am more ambivalent about leaving pipe_tasks in there, but with the helpful update of the ReadMe file I suspect even that should go.

        Use your best judgement and thank you for your work on this valuable tool.

        Show
        rowen Russell Owen added a comment - Thank you for adding pipe_tasks as an optional dependency. Now that I see it I am a bit uneasy about the duplication between the ReadMe file and the ups table file. I naively expected that you would add pipe_tasks to the table file but not the ReadMe. Yet the ReadMe has always had redundancies, since it asks the user to setup the desired obs_ package and validation_data_ package. Furthermore, it makes a great training tool for how to run processCcd.py and other tasks. So asking the user to explicitly set up the pipe_tasks in addition to the correct obs_ package and, in this case, validation_data_ package is clearly a good idea. So I like the new ReadMe. However, I'm nervous about the existing and new redundancy between the ReadMe and the eups table file. I lean towards removing obs_ and validation_data_ from the ups table file. I am more ambivalent about leaving pipe_tasks in there, but with the helpful update of the ReadMe file I suspect even that should go. Use your best judgement and thank you for your work on this valuable tool.
        Hide
        wmwood-vasey Michael Wood-Vasey added a comment -

        Merged to master.

        Show
        wmwood-vasey Michael Wood-Vasey added a comment - Merged to master.

          People

          • Assignee:
            wmwood-vasey Michael Wood-Vasey
            Reporter:
            wmwood-vasey Michael Wood-Vasey
            Reviewers:
            Russell Owen
            Watchers:
            Michael Wood-Vasey, Russell Owen
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: