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

Add documentation strings to Mask plane definitions

    XMLWordPrintable

    Details

    • Story Points:
      28
    • Sprint:
      AP F23-3 (August), AP F23-4 (September)
    • Team:
      Alert Production
    • Urgent?:
      No

      Description

      We need a way to be able to include documentation strings for mask planes in lsst.afw.image.Mask objects, and we probably need a way to define and persist a mask-plane-schema (for lack of a better word) without actually constructing a Mask, just as we define and persist Catalog schemas.

      This will require some C++ work down in afw; it should be possible to do this in a backwards-compatible way, but I suspect it will highlight some existing methods and patterns that we may want to deprecate (some for confusion-avoidance, some to help ensure mask plane definitions are consistent across data IDs).

        Attachments

          Issue Links

            Activity

            Hide
            jbosch Jim Bosch added a comment -

            Notes to John Parejko and my future self from pair programming 2023-03-29:

            We'll want to deprecate all Mask constructors that take MaskPlaneDict, remove the default value for the MaskPlaneDict argument, and add a new variants of those constructors that instead take a std::shared_ptr<afw::image::detail::MaskDict> that defaults to nullptr.

            For the first phase of this, we can provide just a collections.abc.Mapping pybind11 interface to afw::image::detail::MaskDict, and make the Python version of Mask.getMaskPlaneDict return that.

            The Mapping-interface wrapper will preserve all Python usage of getMaskPlaneDict that treats the result as a snapshot just to be looked at but not modified.

            By adding the new constructors we'll also preserve all Python usage of getMaskPlaneDict that just wants to pass the plane definitions from one Mask to another without modification.

            Python usage of getMaskPlaneDict that tries to modify the result will intentionally break; this would include wholly-broken code that thinks it's modifying a view instead of a snapshot and possibly-valid code that's just trying to transfer modified mask plane definitions from one Mask to another. If there's a lot of the latter, we can extend the Mapping-interface wrappers a bit to support some mutation, but we'll also need to make sure such code first copies the dict (since at present it's already modifying a copy) and that it pays attention to docstrings.

            Show
            jbosch Jim Bosch added a comment - Notes to John Parejko and my future self from pair programming 2023-03-29: We'll want to deprecate all Mask constructors that take MaskPlaneDict , remove the default value for the MaskPlaneDict argument, and add a new variants of those constructors that instead take a std::shared_ptr<afw::image::detail::MaskDict> that defaults to nullptr . For the first phase of this, we can provide just a collections.abc.Mapping pybind11 interface to afw::image::detail::MaskDict , and make the Python version of Mask.getMaskPlaneDict return that. The Mapping -interface wrapper will preserve all Python usage of getMaskPlaneDict that treats the result as a snapshot just to be looked at but not modified. By adding the new constructors we'll also preserve all Python usage of getMaskPlaneDict that just wants to pass the plane definitions from one Mask to another without modification. Python usage of getMaskPlaneDict that tries to modify the result will intentionally break; this would include wholly-broken code that thinks it's modifying a view instead of a snapshot and possibly-valid code that's just trying to transfer modified mask plane definitions from one Mask to another. If there's a lot of the latter, we can extend the Mapping -interface wrappers a bit to support some mutation, but we'll also need to make sure such code first copies the dict (since at present it's already modifying a copy) and that it pays attention to docstrings.
            Hide
            Parejkoj John Parejko added a comment -

            Note to self: this discussion on #dm-pybind11 is where this work had previously gotten stuck: https://lsstc.slack.com/archives/C31M66U9L/p1680912396170559

            Show
            Parejkoj John Parejko added a comment - Note to self: this discussion on #dm-pybind11 is where this work had previously gotten stuck: https://lsstc.slack.com/archives/C31M66U9L/p1680912396170559

              People

              Assignee:
              Parejkoj John Parejko
              Reporter:
              jbosch Jim Bosch
              Watchers:
              Gregory Dubois-Felsmann, Jim Bosch, John Parejko
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:

                  Jenkins

                  No builds found.