shapelet
pex_logging
pex_config (maybe test file)
obs_subaru
meas_extensions_simpleShape
meas_extensions_shapeHSM
meas_extensions_psfex
meas_extensions_photometryKron
meas_base
ip_isr (include ordering)
ip_diffim
display_ds9
coadd_utils
coadd_chisquared
base

Moved to post-merge.

shapelet done

pex_logging done

obs_subaru done

meas_extensions_simpleShape done

meas_extensions_shapeHSM done

meas_extensions_psfex done

meas_extensions_photometryKron done

meas_base done

ip_isr done

display_ds9 done

coadd_utils done

coadd_chisquared done

base done

ip_diffim done

It looks to me as if you have not yet tackled import statements. I hope that can be part of this ticket. Do you need help with it?

Also display_ds9 appears to be missing a .cc wrapper file (I hope you just forgot to push it after renaming it from `_xpa.cc` to `xpa.cc`).

I indeed left out the import statements. However, due to DM-9786 it seems likely that we will not need them (or at least not for cross imports). The reason is that we will likely switch to single (fat) modules to reduce the total build size (any further discussion on this point on that ticket please).

I added the renamed file back as requested.

Thanks!

OK. I'll accept that argument about imports, but even if we combine things we still need to import packages required by the pybind11 wrappers, for safety – whether that is done in __init__.py files or as pybind11 import statements. What shall we do about those imports? We must use pybind11 imports for pure pybind11 extensions that are intended to be usable standalone by code that is part of the same product. We can put the imports into __init__.py in other cases and I slightly lean towards that for simplicity (it makes them easier to find and modify). On the other hand putting imports in wrappers arguably gives cleaner encapsulation. Do Jim Bosch, Krzysztof Findeisen or Fred Moolekamp have an opinion?

I still think they ought to go in the pybind11 files. But won't need nearly as many of them once we merge things into larger modules, because all of the intra-package dependencies will already be accounted for.

My understanding is that this cleanup pass is an intermediate stage: the final stage will be to mostly combined the small wrappers into larger wrappers. That is stated as the justification for not having pybind11 import statements in the wrappers. But I am not very happy about this. I would much have this code be correct, with the imports in place so that the modules are self-contained, as we expect them to be. That way we can take our time about the final cleanup pass.

But even if we do decide to go for a quick and dirty solution for imports, for now, something needs to be done because as far as I can see many imports packages are not being imported anywhere, not even the __init__.py files. This makes the code needlessly fragile.

I'll put this back into In Progress.

I added py::module::import statements for all cross package imports that were explicitly needed. Furthermore I changed all from __future__ import lines to the standard as requested.
Finally I verified the build on Jenkins and fixed some minor issues here and there.
Ready for second review.

Overall this looks great. Many of the packages can be merged with no changes. However, there are still a few missing imports in some packages and I found one or two other things.

Merged after implementing comments and CI.
Thanks for the review!