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

Use astro_metadata_translator in obs_lsst gen2 translator

    Details

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

      Description

      Once DM-16554 is merged there will be two metadata translators in obs_lsst. This ticket will replace the translation logic in the butler gen2 translator with astro_metadata_translator logic. It looks like I can subclass ParseTask and insert an ObservationInfo constructor. Then most of the translators can be replaced with a trivial data type conversion rather than duplicated camera logic.

        Attachments

          Issue Links

            Activity

            Hide
            tjenness Tim Jenness added a comment -

            Robert Lupton would you be able to review this ticket? This ticket means that gen2 butler, gen3 butler, and VisitInfo translations are all in a single location.

            Show
            tjenness Tim Jenness added a comment - Robert Lupton would you be able to review this ticket? This ticket means that gen2 butler, gen3 butler, and VisitInfo translations are all in a single location.
            Hide
            tjenness Tim Jenness added a comment -

            Merlin Fisher-Levine thanks for your review. I've fixed the main issues. Outstanding issues are:

            • Should I inherit the AuxTel wavelength calculation from the base class?
            • Simon Krughoff, it might be less confusing if we regenerate the test repository inside obs_lsst so that it uses the updated algorithm for visit ID. I'm not sure how to do that though.

            Now that things are unified I think I might be including one zero too many when forming the exposure Ids from the day number and the sequence number. I think we need space for 99,999 images on a day, not 999,999. I have also included an extra leading zero (as you requested previously) in the detector exposure ID but maybe we shouldn't be making it significantly larger than it needs to be.

            PS Can you please mark the ticket as reviewed.

            Show
            tjenness Tim Jenness added a comment - Merlin Fisher-Levine thanks for your review. I've fixed the main issues. Outstanding issues are: Should I inherit the AuxTel wavelength calculation from the base class? Simon Krughoff , it might be less confusing if we regenerate the test repository inside obs_lsst so that it uses the updated algorithm for visit ID. I'm not sure how to do that though. Now that things are unified I think I might be including one zero too many when forming the exposure Ids from the day number and the sequence number. I think we need space for 99,999 images on a day, not 999,999. I have also included an extra leading zero (as you requested previously) in the detector exposure ID but maybe we shouldn't be making it significantly larger than it needs to be. PS Can you please mark the ticket as reviewed.
            Hide
            mfisherlevine Merlin Fisher-Levine added a comment -

            I've now responded to everything on the PR, except the final issue of IDs in the tests, and the bit about that and calib IDs too, as I don't think any input is necessary from me, and I'm not sure how to fix it.

            I think it's fair to say that it's beyond the scope of this ticket though, and so you should feel free to go ahead and merge, but maybe file a ticket for sorting that stuff out.

            Show
            mfisherlevine Merlin Fisher-Levine added a comment - I've now responded to everything on the PR, except the final issue of IDs in the tests, and the bit about that and calib IDs too, as I don't think any input is necessary from me, and I'm not sure how to fix it. I think it's fair to say that it's beyond the scope of this ticket though, and so you should feel free to go ahead and merge, but maybe file a ticket for sorting that stuff out.

              People

              • Assignee:
                tjenness Tim Jenness
                Reporter:
                tjenness Tim Jenness
                Reviewers:
                Merlin Fisher-Levine, Robert Lupton
                Watchers:
                Colin Slater, Kian-Tat Lim, Merlin Fisher-Levine, Robert Lupton, Simon Krughoff, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel