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

Add Versioning to SourceTable in lsst::afw::table

    XMLWordPrintable

    Details

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

      Description

      Add version to afw::table::SourceTable. Persist that version number to fits file when the table is saved, and restore when the table is restored.

      Tables created and saved to disk prior to this modification will have the version number 0, by default. Tables created with the S14 version will have the version number 1.

      This change is to enable a new version of slots and field naming conventions as needed by the Measurement Framework overhaul, at the same time allowing current clients of SourceTable to continue to function. The work to define and persist the slots depending on the version will be on a separate issue.

      Should not appear as an alterable member of the metadata, but should be saved with the metadata and reloaded when the file is reloaded.

      getVersion and setVersion methods will be used to allow clients to alter this number.

        Attachments

          Issue Links

            Activity

            Hide
            pgee Perry Gee added a comment -

            I think you should review this again to be sure the sequencing in _readtable and _startRecords is OK.

            I was not able to reproduce the metadata problem when I got done, and no longer have the failure case. I assume there was a typo in my code which I fixed without know it.

            Show
            pgee Perry Gee added a comment - I think you should review this again to be sure the sequencing in _readtable and _startRecords is OK. I was not able to reproduce the metadata problem when I got done, and no longer have the failure case. I assume there was a typo in my code which I fixed without know it.
            Hide
            jbosch Jim Bosch added a comment -

            If you want to send a ticket back to review in the future, please set the state back to "In Review". "Complete" is for when it's merged to master.

            Show
            jbosch Jim Bosch added a comment - If you want to send a ticket back to review in the future, please set the state back to "In Review". "Complete" is for when it's merged to master.
            Hide
            jbosch Jim Bosch added a comment -

            Looks good!

            I've just got one more nit-picky comment: in _startRecords, the versioning stuff is protected by an "if (metadata)" block, while the removal of the type key is not, even thought they both rely on the metadata being nonnull. I think we guarantee that the metadata is present here (more precisely, any table subclass whose _readTable() doesn't set the metadata before calling _startRecords should be considered buggy), so I think it'd be fine to remove the if statement, and perhaps replace it with an assert(metadata).

            Show
            jbosch Jim Bosch added a comment - Looks good! I've just got one more nit-picky comment: in _startRecords, the versioning stuff is protected by an "if (metadata)" block, while the removal of the type key is not, even thought they both rely on the metadata being nonnull. I think we guarantee that the metadata is present here (more precisely, any table subclass whose _readTable() doesn't set the metadata before calling _startRecords should be considered buggy), so I think it'd be fine to remove the if statement, and perhaps replace it with an assert(metadata).
            Hide
            pgee Perry Gee added a comment -

            Right. After carefully coding the VERSION stuff, I just kind of stuffed the AFW_TYPE code in with cut and paste. I have now moved this code inside the if(metadata) block.

            Show
            pgee Perry Gee added a comment - Right. After carefully coding the VERSION stuff, I just kind of stuffed the AFW_TYPE code in with cut and paste. I have now moved this code inside the if(metadata) block.
            Hide
            jbosch Jim Bosch added a comment -

            Sounds good. Feel free to merge to master whenever you'd like.

            Show
            jbosch Jim Bosch added a comment - Sounds good. Feel free to merge to master whenever you'd like.

              People

              Assignee:
              pgee Perry Gee
              Reporter:
              pgee Perry Gee
              Reviewers:
              Jim Bosch
              Watchers:
              Jim Bosch, Perry Gee, Robert Lupton
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Due:
                Created:
                Updated:
                Resolved:

                  Time Tracking

                  Estimated:
                  Original Estimate - 6 hours
                  6h
                  Remaining:
                  Remaining Estimate - 1 hour
                  1h
                  Logged:
                  Time Spent - Not Specified Time Not Required
                  Not Specified

                    Jenkins

                    No builds found.