Details
-
Type:
Story
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: butler
-
Labels:None
-
Story Points:6
-
Epic Link:
-
Sprint:DRP S17-2, DRP S17-3
-
Team:Data Release Production
Description
The Butler uses daf.persistence.ReadProxy to support lazy reading, returning these objects in place of things like SourceCatalog and Exposure. In Python, these proxies behave almost entirely like their subjects, because they forward all attributes, but they can't be converted to their C++ counterparts through pybind11's type conversion system. This worked with Swig essentially due to luck: Swig stores its C++ instances in a this attribute that's publicly accessible from Python, so it automatically invokes the proxy's attribute forwarding when trying to do conversions.
I think there are essentially three approaches to dealing with this in pybind11:
- We could remove ReadProxy entirely. A few of us have long been annoyed with them, and we're not yet getting any use out of the lazy loading functionality. We do anticipate needing lazy loading someday, however, and it looks like an RFC to do this would be more contentious and more work (to design a replacement interface for lazy loading) than I originally anticipated.
- We could live with the conversion failure, making Python code that calls wrapped C++ routines call ReadProxy.__subject__ directly (or via a new convenience function) and making the error message when conversions fail much more helpful (e.g. by changing ReadProxy.__repr__. I really don't have a good sense for how much Python we'd have to modify if we take this approach.
- We could add a pybind11 type_caster that does the dereference for any types that could be proxied. If it's possible to add a new type_caster for a class that's already by wrapped with pybind11::class_, and to delegate within it to the original converters defined by pybind11::class_, this will be easy and the course we should take. I'm hoping Pim Schellart [X] knows if it's possible (my best guess is that it is, but it will require using some undocumented and possible internal parts of pybind11).
This ticket is for exploratory work and making a decision (at least with the story points it has now). It should be updated or a new ticket spawned if we identify an approach that will require significant further work.
Some comments on the GitHub PR, but overall I like this quite a bit. Very good idea to add a base class for ReadProxy.