DM-25373 Add Psf trampoline

[jmeyers314]

No description provided.

[kfindeisen]

Looks good in general, though I'd like to discuss the clone vs. cloneStorable and the Psf constructor further before approving.

[jmeyers314]

Looks good in general, though I'd like to discuss the clone vs. cloneStorable and the Psf constructor further before approving.

Thanks, @kfindeisen . I think I've addressed most of your comments now; though still thinking about clone/cloneStorable/__deepcopy__. I added a fixit commit here if you want to take a look (I'll squash it before merging to master).

One additional thing that occurred to me though is that I probably need to follow the instructions for inheriting from Persistable too. I suppose this would mean a few more trampolines for either Persistable or maybe PersistableFacade<>, and probably for PersistableFactory too. Does that sound right?

[kfindeisen]

I'd recommend against it. PersistableFacade has some problems with pybind11 because it tries to have covariant input types; this is particularly true if you have multiple PersistableFacade instantiations in the inheritance tree. More generally, Persistable is fundamentally incompatible with Python (for reasons I don't quite recall, but @TallJimbo probably does), so there's no point supporting Python implementations of it.

[kfindeisen]

Looks good, though I'd like a bit more documentation.

[jmeyers314]

I'd recommend against it. PersistableFacade has some problems with pybind11 because it tries to have covariant input types; this is particularly true if you have multiple PersistableFacade instantiations in the inheritance tree. More generally, Persistable is fundamentally incompatible with Python (for reasons I don't quite recall, but @TallJimbo probably does), so there's no point supporting Python implementations of it.

Hmm... Is this going to cause a problem? I'm not familiar enough to know, but I would have guessed that say, CoaddPsf persistence is implemented via the persistence of its constituents. If that's true, then does this mean pure-python Psfs cannot be persisted as part of a CoaddPsf?

[TallJimbo]

does this mean pure-python Psfs cannot be persisted as part of a CoaddPsf?

That's correct, and yes, that's definitely going to be a problem at some point. I don't know what the best approach is going to be, but it shouldn't get in the way of this ticket, at least.

[jmeyers314]

That's correct, and yes, that's definitely going to be a problem at some point. I don't know what the best approach is going to be, but it shouldn't get in the way of this ticket, at least.

I wonder if I can define write in the trampoline class and then access it in python via super() maybe? But point taken, won't delay this ticket on this account.

[TallJimbo]

The main problem is that all of the other classes that any afw::table::io::Persistable implementation would need to use are not wrapped for Python, and I've never looked hard at what it would take to wrap them; I suspect it would require at least some changes to how the C++ works.