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

Add Gen3 Instrument and Formatter classes for obs_lsst

    Details

    • Story Points:
      4
    • Sprint:
      BG3_F18_10, BG3_F18_11, BG3_S19_01, Arch 2019-07-15, Arch 2019-07-22, Arch 2019-07-29
    • Team:
      Architecture

      Description

      I anticipate this involving some changes to the Instrument base class, and hence changes to our only existing implementation of it in obs_subaru as well.

       

        Attachments

          Issue Links

            Activity

            Hide
            tjenness Tim Jenness added a comment -

            Jim Bosch shall I take this ticket over so you can review it?

            Show
            tjenness Tim Jenness added a comment - Jim Bosch shall I take this ticket over so you can review it?
            Hide
            jbosch Jim Bosch added a comment -

            Please do.

            Show
            jbosch Jim Bosch added a comment - Please do.
            Hide
            tjenness Tim Jenness added a comment -

            Jim Bosch this should probably be reviewed now so that we understand how it relates to DM-20763. John Parejko do you notice anything that will be affected by your ticket?

            All tests pass except the crosstalk assembly test which is marked as an expected failure and which we agreed on slack was outside the scope of this ticket.

            Show
            tjenness Tim Jenness added a comment - Jim Bosch this should probably be reviewed now so that we understand how it relates to DM-20763 . John Parejko do you notice anything that will be affected by your ticket? All tests pass except the crosstalk assembly test which is marked as an expected failure and which we agreed on slack was outside the scope of this ticket.
            Hide
            Parejkoj John Parejko added a comment -

            I suggest holding off on review of this until DM-20154 is merged, so that we can refactor it to use the new WCS code defined there. It might also be worth waiting for DM-20763, so we can use whatever cleanups of Instrument and Formatters come out of that.

            Show
            Parejkoj John Parejko added a comment - I suggest holding off on review of this until DM-20154 is merged, so that we can refactor it to use the new WCS code defined there. It might also be worth waiting for DM-20763 , so we can use whatever cleanups of Instrument and Formatters come out of that.
            Hide
            jbosch Jim Bosch added a comment -

            Fine with me; Tim Jenness, please ping me if you disagree or when those conditions are met.

            Show
            jbosch Jim Bosch added a comment - Fine with me; Tim Jenness , please ping me if you disagree or when those conditions are met.
            Hide
            tjenness Tim Jenness added a comment -

            Jim Bosch I think it's ready for another review. I've added the formatters and the instrument classes and cleaned up assembly some more to ensure gen2 and gen3 share code.

            Some open issues:

            • getCamera is now a class method because I want to cache the yaml camera given how long it takes to be created. I also see that we moved from a property to a getter when John Parejko refactored instrument a while back.
            • I'm a bit concerned about the requirement that detector_exposure_id in gen3 packer matches gen2 and matches astro_metadata_translator. The instrument classes now query astro_metadata_translator for max exposure id and max detector.
            Show
            tjenness Tim Jenness added a comment - Jim Bosch I think it's ready for another review. I've added the formatters and the instrument classes and cleaned up assembly some more to ensure gen2 and gen3 share code. Some open issues: getCamera is now a class method because I want to cache the yaml camera given how long it takes to be created. I also see that we moved from a property to a getter when John Parejko refactored instrument a while back. I'm a bit concerned about the requirement that detector_exposure_id in gen3 packer matches gen2 and matches astro_metadata_translator. The instrument classes now query astro_metadata_translator for max exposure id and max detector.
            Hide
            tjenness Tim Jenness added a comment -

            Christopher Waters would you be able to do this review? Some of it does affect code you are working on.

            John Parejko would you please be able to look at the obs_base changes?

            Show
            tjenness Tim Jenness added a comment - Christopher Waters would you be able to do this review? Some of it does affect code you are working on. John Parejko would you please be able to look at the obs_base changes?
            Hide
            tjenness Tim Jenness added a comment - - edited

            Finally merged this.

            Christopher Waters sorry but you'll have to deal with a conflict.

            Show
            tjenness Tim Jenness added a comment - - edited Finally merged this. Christopher Waters sorry but you'll have to deal with a conflict.

              People

              • Assignee:
                tjenness Tim Jenness
                Reporter:
                jbosch Jim Bosch
                Reviewers:
                Christopher Waters
                Watchers:
                Christopher Waters, Hsin-Fang Chiang, Jim Bosch, John Parejko, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel