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

Reimplement ExposureInfo using GenericMap

    XMLWordPrintable

    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

          is blocked by

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-19467 Add C++ iteration to GenericMap

          • Done

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-19575 Add Storable mixin to ExposureInfo components

          • Done

          Improvement - An improvement or enhancement to an existing feature or task. DM-20286 Modernize pybind11 wrappers for afw.table and afw.table.io

          • Done
          is duplicated by

          Improvement - An improvement or enhancement to an existing feature or task. DM-7537 Add a unit test for ExposureInfo

          • Invalid
          relates to

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-27754 Make Exposure formatters more generic

          • To Do

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-20142 Write DMTN on Exposure and persistence work

          • Done

          Improvement - An improvement or enhancement to an existing feature or task. DM-21327 Replace ExposureInfo implementation with homogeneous map

          • Done

            Activity

            Ascending order - Click to sort in descending order
            Hide
            Permalink
            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
            Permalink
            Parejkoj John Parejko added a comment -

            Note: Calib is now PhotoCalib.

            Show
            Parejkoj John Parejko added a comment - Note: Calib is now PhotoCalib .
            Hide
            Permalink
            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
            Permalink
            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
            Permalink
            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
            Permalink
            krzys Krzysztof Findeisen added a comment -

            PRs are afw#480 and meas_extensions_convolved#16.

            Show
            krzys Krzysztof Findeisen added a comment - PRs are afw#480 and meas_extensions_convolved#16 .
            Hide
            Permalink
            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
            Permalink
            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:

                  Jenkins

                  No builds found.