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

Fix kwargs passing (outputPrefix is None error) in validate_drp

    Details

    • Type: Bug
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: validate_drp
    • Labels:
      None

      Description

      DM-11251 inadvertently introduced a bug when running validateDrp on a traditional repo.

      1. Fix bug by calcultaing in run and propagating outputPrefix through correctly.
      2. Optionally decided what to do to handle case of outputPrefix=None in runOneFilter.
      3. In general fix passing of explicit and implicit keywords through the function chain. Specifically this means also fixing the passing of metrics.

        Attachments

          Issue Links

            Activity

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

            Thanks to @rhl for being the first to catch this!

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - Thanks to @rhl for being the first to catch this!
            Hide
            wmwood-vasey Michael Wood-Vasey added a comment -

            I've verified that
            a) running the validate_drp/examples/runCfhtQuickTest.sh is now fixed
            b) running validateDrp.py on a JSON file still works. (this is in the test suite).
            c) Building lsst_ci now works with passing tests. Jenkins Job #25297:
            https://ci.lsst.codes/job/stack-os-matrix/25297/

            I've created an additional ticket to add a test to make sure I don't stupidly break the main purpose of validate_drp again, but as master is currently broken, I suggest we merge this present ticket now even without the test.

            In the future, even without the implementation of DM-11301, I will make sure to do a full lsst_ci run before merging changes to validate_drp master.

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - I've verified that a) running the validate_drp/examples/runCfhtQuickTest.sh is now fixed b) running validateDrp.py on a JSON file still works. (this is in the test suite). c) Building lsst_ci now works with passing tests. Jenkins Job #25297: https://ci.lsst.codes/job/stack-os-matrix/25297/ I've created an additional ticket to add a test to make sure I don't stupidly break the main purpose of validate_drp again, but as master is currently broken, I suggest we merge this present ticket now even without the test. In the future, even without the implementation of DM-11301 , I will make sure to do a full lsst_ci run before merging changes to validate_drp master .
            Hide
            wmwood-vasey Michael Wood-Vasey added a comment - - edited

            Obviously, doing a full lsst_ci run should be always the default. But, I got sloppy. The lsst_ci run I submitted to do the checking for DM-11215 failed because some other thing was failing, so I rationalized myself out of it saying that it was fine. I was clearly wrong excessively optimistic to do so and have been suitably chastened not to do so again.

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - - edited Obviously, doing a full lsst_ci run should be always the default. But, I got sloppy. The lsst_ci run I submitted to do the checking for DM-11215 failed because some other thing was failing, so I rationalized myself out of it saying that it was fine. I was clearly wrong excessively optimistic to do so and have been suitably chastened not to do so again.
            Hide
            wmwood-vasey Michael Wood-Vasey added a comment -

            Also passes on my desktop

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - Also passes on my desktop
            Hide
            rhl Robert Lupton added a comment -

            Looks good. A couple of comments on GitHub

            Show
            rhl Robert Lupton added a comment - Looks good. A couple of comments on GitHub
            Hide
            wmwood-vasey Michael Wood-Vasey added a comment -

            In fix this issue, I realized the more general description was to fix the passing of keyword arguments through the chain of functions. Some are explicit and some are in kwargs, but need to be inspected. I have updated the description of this ticket to include this.

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - In fix this issue, I realized the more general description was to fix the passing of keyword arguments through the chain of functions. Some are explicit and some are in kwargs , but need to be inspected. I have updated the description of this ticket to include this.
            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:
                Robert Lupton
                Watchers:
                Michael Wood-Vasey, Robert Lupton
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel