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

Metaclass for wrapped-template ABCs and class extension decorators

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: utils
    • Labels:
      None
    • Story Points:
      4
    • Sprint:
      DRP S17-3
    • Team:
      Data Release Production

      Description

      Our codebase is full of template classes that are wrapped into a set of Python classes with suffixes derived from their template type (e.g. ImageF, ImageD, ImageU).

      In some places, we've defined a module scoped dictionary with the un-suffixed name to relate these, e.g. afw.table.Key["F"] is afw.table.KeyF.

      To enhance both the usuability of our Python interfaces and the ease of writing wrappers, we should:

      • Add an Abstract Base Class for each of these template classes (https://docs.python.org/2/library/abc.html), making it appear as though all template instantiations inherit from the same class (in e.g. isinstance checks).
      • Make these ABCs inherit from a custom metaclass (which in turn inherits from abc.ABCMeta) that allows dict-like access to the registered instantiations from both the string suffixes and the corresponding numpy types (e.g. numpy.float32 for "F"), preserving the dict behavior mentioned above.
      • Add a __new__ method to each ABC that accepts a dtype argument and forwards all other arguments to the constructor of the wrapped template the dtype key points to. In other words, make e.g. Image(bbox, dtype=numpy.float32) call ImageF(bbox). We could probably have the metaclass inject this automatically.

      New users to the stack should be able to mostly get by without seeing the templated types at all - they'd only interact with the ABC. And devs writing the Python wrappers for templated classes could define pure-Python extensions directly in the ABC instead of monkeypatching them into each wrapped template.

      This issue covers adding the metaclass to utils and demonstrating its use in at least one template class wrapper. Using it for other classes could be done on other pybind11 branches, as it strictly represents an addition to the API, and it could be useful to do it there as to get the monkeypatching-avoidance mentioned above.

      In addition to the above, this ticket should include an implementation for the decorators described in RFC-270, which have significant code in common with the metaclass (as implemented currently) and a related purpose.

        Attachments

          Issue Links

            Activity

            Hide
            jbosch Jim Bosch added a comment -

            Krzysztof Findeisen, thanks for the quick reply.

            So, we've had a request for a Python metaprogramming expert. I wonder if Nate Lust is up for a review

            Show
            jbosch Jim Bosch added a comment - Krzysztof Findeisen , thanks for the quick reply. So, we've had a request for a Python metaprogramming expert. I wonder if Nate Lust is up for a review
            Hide
            nlust Nate Lust added a comment -

            I put up a bunch of review comments, if you have any questions please ask. I would like to see some of them addressed or at least discussed before I mark this as reviewed.

            Show
            nlust Nate Lust added a comment - I put up a bunch of review comments, if you have any questions please ask. I would like to see some of them addressed or at least discussed before I mark this as reviewed.
            Hide
            jbosch Jim Bosch added a comment -

            I believe I have now addressed all review comments.

            Show
            jbosch Jim Bosch added a comment - I believe I have now addressed all review comments.
            Hide
            jbosch Jim Bosch added a comment -

            As a side note, I am now seriously considering a __getattr__/__setattr__ approach to making TemplateMeta attributes appear in registered subclasses, but not on this ticket; that will require real thought and care to do properly, and probably isn't worth the effort right now. Aside from fixing the mro so that ABC methods no longer override subclass methods, the big motivation is making it possible to have the ABC inherit from another class. This would be useful in e.g. afw.image where many classes Exposure, MaskedImage, Mask, Image all share some pure-Python methods from slicing.py that they could be inheriting from a common base class that sits above their template ABCs. I can work around that just fine for now, though.

            Show
            jbosch Jim Bosch added a comment - As a side note, I am now seriously considering a __getattr__ / __setattr__ approach to making TemplateMeta attributes appear in registered subclasses, but not on this ticket; that will require real thought and care to do properly, and probably isn't worth the effort right now. Aside from fixing the mro so that ABC methods no longer override subclass methods, the big motivation is making it possible to have the ABC inherit from another class. This would be useful in e.g. afw.image where many classes Exposure , MaskedImage , Mask , Image all share some pure-Python methods from slicing.py that they could be inheriting from a common base class that sits above their template ABCs. I can work around that just fine for now, though.
            Hide
            jbosch Jim Bosch added a comment -

            Merged to tickets/DM-8467.

            Show
            jbosch Jim Bosch added a comment - Merged to tickets/ DM-8467 .

              People

              Assignee:
              jbosch Jim Bosch
              Reporter:
              jbosch Jim Bosch
              Reviewers:
              Nate Lust
              Watchers:
              David Shupe, Jim Bosch, Nate Lust, Pim Schellart [X] (Inactive), Tim Jenness
              Votes:
              1 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  CI Builds

                  No builds found.