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

Create focus script

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: obs_subaru
    • Labels:
      None
    • Story Points:
      2
    • Sprint:
      DRP X16-3, DRP F16-1, DRP F16-2, DRP F16-3
    • Team:
      Data Release Production

      Description

      In DM-3368, we stripped out the focus calculation since it's not camera-generic, and the scatter/gather isn't necessary for general processing. We need to reinstate the focus calculation in its own scatter/gather script.

        Attachments

          Activity

          Hide
          rearmstr Bob Armstrong added a comment -

          I can't seem to create a pull request so I will post my comments here:

          My feeling is that it does not have to conform to LSST standards in the initial commit here. But if we are going to continue to use this, I would really like to clean this up a bit and add more documentation. My only concern with not doing it now is that it will never get done. Maybe we can reach out to Hiranao to improve this a bit.

          In ups/hscPipeline.table:

          • is there a reason to have the optional dependecies that it doesn't use? The only one it uses is simpleShape and I think that should be required because it won't work without it.

          In FocusTask.py:

          • Can we remove the commented out code on lines 47, 56, 59
          • I don't understand the comment on line 52. Isn't this already written using the current measurement framework?
          • Remove extraneous print statement line 55

          In focus.py:

          • Not crazy about the variable names in getCorrectedFocusError. But, maybe this can be pushed to a general cleanup of the code.
          Show
          rearmstr Bob Armstrong added a comment - I can't seem to create a pull request so I will post my comments here: My feeling is that it does not have to conform to LSST standards in the initial commit here. But if we are going to continue to use this, I would really like to clean this up a bit and add more documentation. My only concern with not doing it now is that it will never get done. Maybe we can reach out to Hiranao to improve this a bit. In ups/hscPipeline.table: is there a reason to have the optional dependecies that it doesn't use? The only one it uses is simpleShape and I think that should be required because it won't work without it. In FocusTask.py: Can we remove the commented out code on lines 47, 56, 59 I don't understand the comment on line 52. Isn't this already written using the current measurement framework? Remove extraneous print statement line 55 In focus.py: Not crazy about the variable names in getCorrectedFocusError. But, maybe this can be pushed to a general cleanup of the code.
          Hide
          rearmstr Bob Armstrong added a comment -

          I can't seem to create a pull request so I will post my comments here:

          My feeling is that it does not have to conform to LSST standards in the initial commit here. But if we are going to continue to use this, I would really like to clean this up a bit and add more documentation. My only concern with not doing it now is that it will never get done. Maybe we can reach out to Hiranao to improve this a bit.

          In ups/hscPipeline.table:

          • is there a reason to have the optional dependecies that it doesn't use? The only one it uses is simpleShape and I think that should be required because it won't work without it.

          In FocusTask.py:

          • Can we remove the commented out code on lines 47, 56, 59
          • I don't understand the comment on line 52. Isn't this already written using the current measurement framework?
          • Remove extraneous print statement line 55

          In focus.py:

          • Not crazy about the variable names in getCorrectedFocusError. But, maybe this can be pushed to a general cleanup of the code.
          Show
          rearmstr Bob Armstrong added a comment - I can't seem to create a pull request so I will post my comments here: My feeling is that it does not have to conform to LSST standards in the initial commit here. But if we are going to continue to use this, I would really like to clean this up a bit and add more documentation. My only concern with not doing it now is that it will never get done. Maybe we can reach out to Hiranao to improve this a bit. In ups/hscPipeline.table: is there a reason to have the optional dependecies that it doesn't use? The only one it uses is simpleShape and I think that should be required because it won't work without it. In FocusTask.py: Can we remove the commented out code on lines 47, 56, 59 I don't understand the comment on line 52. Isn't this already written using the current measurement framework? Remove extraneous print statement line 55 In focus.py: Not crazy about the variable names in getCorrectedFocusError. But, maybe this can be pushed to a general cleanup of the code.
          Hide
          rearmstr Bob Armstrong added a comment -

          I can't seem to create a pull request so I will post my comments here:

          My feeling is that it does not have to conform to LSST standards in the initial commit here. But if we are going to continue to use this, I would really like to clean this up a bit and add more documentation. My only concern with not doing it now is that it will never get done. Maybe we can reach out to Hiranao to improve this a bit.

          In ups/hscPipeline.table:

          • is there a reason to have the optional dependecies that it doesn't use? The only one it uses is simpleShape and I think that should be required because it won't work without it.

          In FocusTask.py:

          • Can we remove the commented out code on lines 47, 56, 59
          • I don't understand the comment on line 52. Isn't this already written using the current measurement framework?
          • Remove extraneous print statement line 55

          In focus.py:

          • Not crazy about the variable names in getCorrectedFocusError. But, maybe this can be pushed to a general cleanup of the code.
          Show
          rearmstr Bob Armstrong added a comment - I can't seem to create a pull request so I will post my comments here: My feeling is that it does not have to conform to LSST standards in the initial commit here. But if we are going to continue to use this, I would really like to clean this up a bit and add more documentation. My only concern with not doing it now is that it will never get done. Maybe we can reach out to Hiranao to improve this a bit. In ups/hscPipeline.table: is there a reason to have the optional dependecies that it doesn't use? The only one it uses is simpleShape and I think that should be required because it won't work without it. In FocusTask.py: Can we remove the commented out code on lines 47, 56, 59 I don't understand the comment on line 52. Isn't this already written using the current measurement framework? Remove extraneous print statement line 55 In focus.py: Not crazy about the variable names in getCorrectedFocusError. But, maybe this can be pushed to a general cleanup of the code.
          Hide
          price Paul Price added a comment -

          FINALLY got back to this (sorry for the delay).

          I've cleaned out the table file and fixed up the focusTask.py problems. I haven't attempted to clean up any of focus.py.

          Put on master of hscPipeline; at the moment this only lives in my GitHub account, but we can move it later.

          Show
          price Paul Price added a comment - FINALLY got back to this (sorry for the delay). I've cleaned out the table file and fixed up the focusTask.py problems. I haven't attempted to clean up any of focus.py. Put on master of hscPipeline; at the moment this only lives in my GitHub account, but we can move it later.
          Hide
          swinbank John Swinbank added a comment -

          Note to self – since we aren't actually using this on LSST, it doesn't need to go in release notes.

          Show
          swinbank John Swinbank added a comment - Note to self – since we aren't actually using this on LSST, it doesn't need to go in release notes.

            People

            • Assignee:
              price Paul Price
              Reporter:
              price Paul Price
              Reviewers:
              Bob Armstrong
              Watchers:
              Bob Armstrong, John Swinbank, Paul Price
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Summary Panel