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

Add Gen3 Instrument and Formatter classes for obs_lsst

    XMLWordPrintable

    Details

    • Story Points:
      4
    • Epic Link:
    • 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

          is blocked by

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-20763 Add initial support for Gen3 Butler to obs_decam

          • Done

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-20812 Rename TablePersistable storage classes in gen 3 to drop TablePersistable

          • Done

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-21357 Add items() method to PropertySet/PropertyList

          • Done

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-16412 Detectors obtained from Camera have wrong amplifer bboxes for untrimmed+assembled in obs_lsst

          • Won't Fix
          is triggering

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-20994 Write how to make gen3 obs package guide

          • Done
          mentioned in

          Page Loading...

            Activity

            Ascending order - Click to sort in descending order
            Hide
            Permalink
            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
            Permalink
            jbosch Jim Bosch added a comment -

            Please do.

            Show
            jbosch Jim Bosch added a comment - Please do.
            Hide
            Permalink
            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
            Permalink
            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
            Permalink
            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
            Permalink
            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
            Permalink
            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
            Permalink
            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:

                  Jenkins

                  No builds found.