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

Add InstrumentLabel to new exposures

    XMLWordPrintable

    Details

    • Story Points:
      3
    • Sprint:
      AP F20-6 (November), AP S21-2 (January)
    • Team:
      Alert Production
    • Urgent?:
      No

      Description

      Once an instrument component exists for exposures, populate it with the value of Instrument.getName() anywhere where observatory code assigns a filter to an exposure.

        Attachments

          Issue Links

            Activity

            No builds found.
            krzys Krzysztof Findeisen created issue -
            krzys Krzysztof Findeisen made changes -
            Field Original Value New Value
            Link This issue is triggered by RFC-730 [ RFC-730 ]
            krzys Krzysztof Findeisen made changes -
            Link This issue is blocked by DM-27174 [ DM-27174 ]
            krzys Krzysztof Findeisen made changes -
            Story Points 4 3
            tjenness Tim Jenness made changes -
            Labels filter-remake gen3-middleware SciencePipelines filter-remake gen3-middleware
            krzys Krzysztof Findeisen made changes -
            Assignee Krzysztof Findeisen [ krzys ]
            krzys Krzysztof Findeisen made changes -
            Epic Link DM-26803 [ 439755 ]
            krzys Krzysztof Findeisen made changes -
            Sprint AP F20-6 (November) [ 1052 ]
            Team Alert Production [ 10300 ]
            sullivan Ian Sullivan made changes -
            Sprint AP F20-6 (November) [ 1052 ] AP F20-6 (November), AP S21-1 (December) [ 1052, 1059 ]
            krzys Krzysztof Findeisen made changes -
            Sprint AP F20-6 (November), AP S21-1 (December) [ 1052, 1059 ] AP F20-6 (November), AP S21-2 (January) [ 1052, 1064 ]
            sullivan Ian Sullivan made changes -
            Epic Link DM-26803 [ 439755 ] DM-27911 [ 442604 ]
            Hide
            Parejkoj John Parejko added a comment -

            Tim Jenness says on DM-27174: ":to propagate the instrument label to all products it's a one line patch to https://github.com/lsst/obs_base/blob/master/python/lsst/obs/base/makeRawVisitInfoViaObsInfo.py#L106". However, the ObservationInfo object does not have an Instrument that we can call getName() on, and I don't know that I trust that e.g. the "INSTRUM" header (or whatever it is in FITS) will match Instrument.getName(). There is a TestMakeRawVisitInfoViaObsInfo to test it, if we go that route. VisitInfo is immutable, so we can't populate it after the fact.

            Show
            Parejkoj John Parejko added a comment - Tim Jenness says on DM-27174 : ":to propagate the instrument label to all products it's a one line patch to https://github.com/lsst/obs_base/blob/master/python/lsst/obs/base/makeRawVisitInfoViaObsInfo.py#L106 ". However, the ObservationInfo object does not have an Instrument that we can call getName() on, and I don't know that I trust that e.g. the "INSTRUM" header (or whatever it is in FITS) will match Instrument.getName() . There is a TestMakeRawVisitInfoViaObsInfo to test it, if we go that route. VisitInfo is immutable, so we can't populate it after the fact.
            Hide
            tjenness Tim Jenness added a comment -

            I'm not really following you. The instrument property of ObservationInfo is guaranteed to match the instrument name used by the Instrument class and the gen3 butler. Gen3 butler uses the instrument property when ingesting data so they must match otherwise ingest would fail. Do not use the INSTRUME header directly – the entire purpose of astro_metadata_translator is to give you a standardized instrument name from a FITS header.

            Show
            tjenness Tim Jenness added a comment - I'm not really following you. The instrument property of ObservationInfo is guaranteed to match the instrument name used by the Instrument class and the gen3 butler. Gen3 butler uses the instrument property when ingesting data so they must match otherwise ingest would fail. Do not use the INSTRUME header directly – the entire purpose of astro_metadata_translator is to give you a standardized instrument name from a FITS header.
            Parejkoj John Parejko made changes -
            Assignee Krzysztof Findeisen [ krzys ] John Parejko [ parejkoj ]
            Parejkoj John Parejko made changes -
            Status To Do [ 10001 ] In Progress [ 3 ]
            Hide
            Parejkoj John Parejko added a comment -

            Krzysztof Findeisen and Tim Jenness: do you both mind having a look? This ended up being a 3 line change, including tests.

            Jenkins: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/33420/pipeline

            I did not add any tests of this in obs_base ingest_test.py because it all happens in makeRawVisitInfoViaObsInfo.py and the ingest code doesn't directly affect any of it. Let me know if you think explicit per-obs tests of the name that gets assigned to visitInfo.instrumentLabel are warranted.

            Show
            Parejkoj John Parejko added a comment - Krzysztof Findeisen and Tim Jenness : do you both mind having a look? This ended up being a 3 line change, including tests. Jenkins: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/33420/pipeline I did not add any tests of this in obs_base ingest_test.py because it all happens in makeRawVisitInfoViaObsInfo.py and the ingest code doesn't directly affect any of it. Let me know if you think explicit per-obs tests of the name that gets assigned to visitInfo.instrumentLabel are warranted.
            Parejkoj John Parejko made changes -
            Reviewers Krzysztof Findeisen, Tim Jenness [ krzys, tjenness ]
            Status In Progress [ 3 ] In Review [ 10004 ]
            Hide
            tjenness Tim Jenness added a comment -

            Looks good to me.

            Show
            tjenness Tim Jenness added a comment - Looks good to me.
            krzys Krzysztof Findeisen made changes -
            Status In Review [ 10004 ] Reviewed [ 10101 ]
            Parejkoj John Parejko made changes -
            Resolution Done [ 10000 ]
            Status Reviewed [ 10101 ] Done [ 10002 ]

              People

              Assignee:
              Parejkoj John Parejko
              Reporter:
              krzys Krzysztof Findeisen
              Reviewers:
              Krzysztof Findeisen, Tim Jenness
              Watchers:
              John Parejko, Krzysztof Findeisen, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.