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

Remove exposureId from visitInfo

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Invalid
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: afw

      Description

      exposureId in visitInfo is not a visit-exposure level identifier, but rather a ccd-exposure level identifier (see docstring for obs.base.CameraMapper._computeCcdExposureId() for the only place where this seems to be specified). Thus, it should not be included in visitInfo, which is only things that are common across the visit. exposureId should be moved back into ExposureInfo.

      We do need a visit-exposure level identifier to be included in visitInfo, so that we can link exposures within visits without using the butler (e.g. without dataRef.dataId['visit']). We could have makeRawVisitInfo() add the butler visitId to the new visitInfo.visitId that this ticket will add.

      Marking this Critical because we should fix this ASAP, before use of visitInfo.exposureId becomes widespread. This is an API-breaking change, so it may need an RFC?

      See this discussion that generated this ticket:

      https://lsstc.slack.com/archives/C3UCAEW3D/p1517724549000034

        Attachments

          Issue Links

            Activity

            Hide
            erykoff Eli Rykoff added a comment -

            (a) I wholeheartedly endorse this, but (b) I'm pretty sure it requires an RFC.

            Show
            erykoff Eli Rykoff added a comment - (a) I wholeheartedly endorse this, but (b) I'm pretty sure it requires an RFC.
            Hide
            rowen Russell Owen added a comment -

            Thank you for the RFC. Well done.

            Show
            rowen Russell Owen added a comment - Thank you for the RFC. Well done.
            Hide
            dtaranu Dan Taranu added a comment -

            The RFC has been adopted. John Parejko, do you want to claim credit for this and mark it done?

            Show
            dtaranu Dan Taranu added a comment - The RFC has been adopted. John Parejko , do you want to claim credit for this and mark it done?
            Hide
            tjenness Tim Jenness added a comment -

            The work hasn't been done. RFCs in adopted state are RFCs where we have agreed to do the work but the work is still to be done. Implemented means that the work has been completed.

            Show
            tjenness Tim Jenness added a comment - The work hasn't been done. RFCs in adopted state are RFCs where we have agreed to do the work but the work is still to be done. Implemented means that the work has been completed.
            Hide
            dtaranu Dan Taranu added a comment -

            The RFC has an epic with two additional tickets to implement it. That would seem to make this ticket redundant.

            Show
            dtaranu Dan Taranu added a comment - The RFC has an epic with two additional tickets to implement it. That would seem to make this ticket redundant.
            Hide
            tjenness Tim Jenness added a comment -

            It could be argued that this ticket is a duplicate of DM-13943. Maybe John Parejko wants to comment since he filed this.

            Show
            tjenness Tim Jenness added a comment - It could be argued that this ticket is a duplicate of DM-13943 . Maybe John Parejko wants to comment since he filed this.
            Hide
            Parejkoj John Parejko added a comment -

            Marking this as a duplicate of DM-13943: I think that ticket has the better approach (moving it out of VisitInfo and into ExposureInfo, where it belongs). Doing that will take care of this.

            Show
            Parejkoj John Parejko added a comment - Marking this as a duplicate of DM-13943 : I think that ticket has the better approach (moving it out of VisitInfo and into ExposureInfo, where it belongs). Doing that will take care of this.

              People

              Assignee:
              Unassigned Unassigned
              Reporter:
              Parejkoj John Parejko
              Watchers:
              Dan Taranu, Eli Rykoff, Jim Bosch, John Parejko, John Swinbank, Kian-Tat Lim, Michael Wood-Vasey, Russell Owen, Simon Krughoff, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              10 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.