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

Remove VisitInfo.exposureId

    XMLWordPrintable

    Details

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

      Description

      Remove exposureId from VisitInfo. This will be a breaking change to the VisitInfo persistence format; I suspect the VisitInfo code will need to remember separate schemas for versions 2 and 3 of VisitInfo.

      Assuming DM-13943 is completed before Pipelines version 24, this ticket must be done after release 25 and before release 26.

        Attachments

          Issue Links

            Activity

            Hide
            sullivan Ian Sullivan added a comment -

            We are past release 25, so we should plan this work before release 26.

            Show
            sullivan Ian Sullivan added a comment - We are past release 25, so we should plan this work before release 26.
            Hide
            krzys Krzysztof Findeisen added a comment - - edited

            When was release 25 done? DM-32971 is still open.

            Show
            krzys Krzysztof Findeisen added a comment - - edited When was release 25 done? DM-32971 is still open.
            Hide
            sullivan Ian Sullivan added a comment -

            We are on RC4 of v25. Per DM-32971, v25 will be based on weekly 48 of 2022. So we can make post-v25 changes as long as we don't backport them to the release branches.

            Show
            sullivan Ian Sullivan added a comment - We are on RC4 of v25. Per DM-32971 , v25 will be based on weekly 48 of 2022. So we can make post-v25 changes as long as we don't backport them to the release branches.
            Hide
            Parejkoj John Parejko added a comment -

            I've pushed enough changes to get afw to compile with exposureId removed, but a test fails because it can no longer read older versions of VisitInfo files. I'll keep poking at it (Krzysztof's comment about remembering schema versions is no doubt correct above), but likely won't get much further until Wednesday, so I'm leaving a breadcrumb trail here for someone else to follow.

            https://github.com/lsst/afw/tree/tickets/DM-32138

            Show
            Parejkoj John Parejko added a comment - I've pushed enough changes to get afw to compile with exposureId removed, but a test fails because it can no longer read older versions of VisitInfo files. I'll keep poking at it (Krzysztof's comment about remembering schema versions is no doubt correct above), but likely won't get much further until Wednesday, so I'm leaving a breadcrumb trail here for someone else to follow. https://github.com/lsst/afw/tree/tickets/DM-32138
            Hide
            Parejkoj John Parejko added a comment -

            Final breadcrumbs before I quit for a few days: I've pushed a couple of commits that attempt to add backwards compatibility to VisitInfoSchema, but now it fails when reading the version=5 test file that I wrote (and that appears to have the correct new schema, when viewed with e.g. astropy.io.fits). Jim Bosch: if you do get a chance to look at it and can figure out what I missed (or come up with a better approach), please go ahead. There's also a whole bunch of VisitInfo instantiations that we have to deal with in many different packages, which I haven't done, but can do on Wednesday morning.

            Show
            Parejkoj John Parejko added a comment - Final breadcrumbs before I quit for a few days: I've pushed a couple of commits that attempt to add backwards compatibility to VisitInfoSchema, but now it fails when reading the version=5 test file that I wrote (and that appears to have the correct new schema, when viewed with e.g. astropy.io.fits ). Jim Bosch : if you do get a chance to look at it and can figure out what I missed (or come up with a better approach), please go ahead. There's also a whole bunch of VisitInfo instantiations that we have to deal with in many different packages, which I haven't done, but can do on Wednesday morning.
            Show
            Parejkoj John Parejko added a comment - - edited Jenkins: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/39128/pipeline with ci_hsc/ci_imsim: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/39129/pipeline
            Show
            Parejkoj John Parejko added a comment - Krzysztof Findeisen : thank you for agreeing to review this on short notice. PRs: https://github.com/lsst/afw/pull/697 - there's one open "TODO" question here https://github.com/lsst/ap_association/pull/174 https://github.com/lsst/cp_pipe/pull/203 https://github.com/lsst/obs_base/pull/460 https://github.com/lsst/ip_diffim/pull/269 https://github.com/lsst/jointcal/pull/248 https://github.com/lsst/pipe_tasks/pull/810

              People

              Assignee:
              Parejkoj John Parejko
              Reporter:
              krzys Krzysztof Findeisen
              Reviewers:
              Krzysztof Findeisen
              Watchers:
              Ian Sullivan, Jim Bosch, John Parejko, Krzysztof Findeisen
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.