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

Reimplement ExposureInfo using GenericMap

    Details

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

      Description

      Instead of hard-coding the contained classes, reimplement ExposureInfo using a type-safe heterogeneous map. The existing interface should be left as-is (or, at most, deprecated). This ticket does not include migrating ExposureInfo away from afw::table::io persistence.

        Attachments

          Issue Links

            Activity

            Hide
            krzys Krzysztof Findeisen added a comment -

            Some notes on the ExposureInfo persistence format:

            • ExposureInfo::_startWriteFits is responsible for persistence, ExposureFitsReader for depersistence.
            • the metadata are stored in the header
            • the filter and VisitInfo are stored in the metadata
            • the WCS and Calib may be stored as either a table::io archive component or as metadata, with the former taking priority
            • all other components are stored as archive components
            • all components stored in the archive have their component ID in the header (e.g., SKYWCS_ID or VALID_POLYGON_ID)

            The new code must be able to read old exposure files, and old code must be able to read new exposures that contain only old-style ExposureInfo components.

            Show
            krzys Krzysztof Findeisen added a comment - Some notes on the ExposureInfo persistence format: ExposureInfo::_startWriteFits is responsible for persistence, ExposureFitsReader for depersistence. the metadata are stored in the header the filter and VisitInfo are stored in the metadata the WCS and Calib may be stored as either a table::io archive component or as metadata, with the former taking priority all other components are stored as archive components all components stored in the archive have their component ID in the header (e.g., SKYWCS_ID or VALID_POLYGON_ID ) The new code must be able to read old exposure files, and old code must be able to read new exposures that contain only old-style ExposureInfo components.
            Hide
            Parejkoj John Parejko added a comment -

            Note: Calib is now PhotoCalib.

            Show
            Parejkoj John Parejko added a comment - Note: Calib is now PhotoCalib .
            Hide
            krzys Krzysztof Findeisen added a comment - - edited

            All components have been migrated except for _filter, _metadata, and _visitInfo.

            One last question: do we want to deprecate the old accessors (e.g., getWcs() vs. getComponent(ExposureInfo::KEY_WCS))?

            Show
            krzys Krzysztof Findeisen added a comment - - edited All components have been migrated except for _filter , _metadata , and _visitInfo . One last question: do we want to deprecate the old accessors (e.g., getWcs() vs. getComponent(ExposureInfo::KEY_WCS) )?
            Hide
            Parejkoj John Parejko added a comment -

            That deprecation should probably be an RFC. I'm not sure I'd be a fan such a deprecation: the existing accessors return null pointers if the object doesn't exist, which seems reasonable.

            Show
            Parejkoj John Parejko added a comment - That deprecation should probably be an RFC. I'm not sure I'd be a fan such a deprecation: the existing accessors return null pointers if the object doesn't exist, which seems reasonable.
            Hide
            krzys Krzysztof Findeisen added a comment -

            Hi Jim Bosch, can you please review this ticket? In addition to looking at the technical details, please let me know if this will work with your future plans for ExposureInfo, and how user-friendly the API is. Thanks!

            Show
            krzys Krzysztof Findeisen added a comment - Hi Jim Bosch , can you please review this ticket? In addition to looking at the technical details, please let me know if this will work with your future plans for ExposureInfo , and how user-friendly the API is. Thanks!
            Hide
            krzys Krzysztof Findeisen added a comment -
            Show
            krzys Krzysztof Findeisen added a comment - PRs are afw#480 and meas_extensions_convolved#16 .
            Hide
            jbosch Jim Bosch added a comment -

            Looks great.  A few comments on the PR.

            Show
            jbosch Jim Bosch added a comment - Looks great.  A few comments on the PR.
            Hide
            krzys Krzysztof Findeisen added a comment -

            Thanks for the multiple lengthy reviews! I've put up a community post on the new API.

            Show
            krzys Krzysztof Findeisen added a comment - Thanks for the multiple lengthy reviews! I've put up a community post on the new API.

              People

              • Assignee:
                krzys Krzysztof Findeisen
                Reporter:
                krzys Krzysztof Findeisen
                Reviewers:
                Jim Bosch
                Watchers:
                Jim Bosch, John Parejko, Krzysztof Findeisen
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel