# Modernize pybind11 wrappers for afw.table and afw.table.io

XMLWordPrintable

#### Details

• Type: Improvement
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
None
• Story Points:
6
• Sprint:
AP F19-1
• Team:

#### Description

DM-17982 is blocked on circular import problems in afw (which, strictly speaking, should have been caught in DM-19575). They are only detectable if each unit test script is run by a separate pytest instance (pytest --forked does not work), but are likely to appear unexpectedly for real users. I've identified the following cycles so far:

 python/lsst/afw/typehandling/__init__.py:23: in   from ._typehandling import Storable python/lsst/afw/table/io/__init__.py:23: in   from .persistable import * python/lsst/afw/fits/__init__.py:2: in   from .pickleFits import reduceToFits, unreduceFromFits python/lsst/afw/fits/pickleFits.py:4: in   import lsst.afw.image python/lsst/afw/image/__init__.py:25: in   from .apCorrMap import * python/lsst/afw/image/apCorrMap/__init__.py:23: in   from .apCorrMap import * E ImportError: generic_type: type "ApCorrMap" referenced unknown base type "lsst::afw::typehandling::Storable" 

 python/lsst/afw/typehandling/__init__.py:23: in   from ._typehandling import Storable python/lsst/afw/table/__init__.py:28: in   from .aggregates import * python/lsst/afw/table/aggregates/__init__.py:23: in   from .aggregates import * python/lsst/afw/geom/__init__.py:30: in   from .polygon import * python/lsst/afw/geom/polygon/__init__.py:25: in   from .polygon import * E ImportError: generic_type: type "Polygon" referenced unknown base type "lsst::afw::typehandling::Storable" 

After discussing on #dm-pybind-11, I suspect that the most productive way to deal with the cycles is to migrate the lsst.afw.table.io package and its dependency lsst.afw.table to the lsst::utils::python::WrapperCollection framework. Hopefully doing so will fix the dependency problems associated with inheriting from Storable without requiring complete conversion of all of afw.

#### Activity

Hide
Krzysztof Findeisen added a comment -

I've successfully rewrapped table, table.io, and detection (because modernizing the wrapper for PeakCatalog let me get rid of some duplicate code, which I felt was worth the scope creep).

Show
Krzysztof Findeisen added a comment - I've successfully rewrapped table , table.io , and detection (because modernizing the wrapper for PeakCatalog let me get rid of some duplicate code, which I felt was worth the scope creep).
Hide
Krzysztof Findeisen added a comment -

Hi John Parejko, would you please look over this code? PRs are afw#476 and meas_base#146. Thanks!

If you want to streamline going over all 99 afw files (which were largely the same rename/cut-and-paste/debug cycle), I think the following list should cover the most important bits:

Show
Krzysztof Findeisen added a comment - Hi John Parejko , would you please look over this code? PRs are afw#476 and meas_base#146 . Thanks! If you want to streamline going over all 99 afw files (which were largely the same rename/cut-and-paste/debug cycle), I think the following list should cover the most important bits: table/python/*.h ( bf2c8d2 and 7817986 ) BaseCatalog , SimpleCatalog , and SourceCatalog ( b61dd0e and 5fdd793 ) AmpInfoCatalog (part of f3b80c3 ) Psf ( fe3e0a8 ) Footprint (part of c9141fa ) PeakCatalog ( ce82910 )
Hide
John Parejko added a comment -

I looked over a selection of the commits (the ones you listed, and a few more). Beyond the few comments I made, I'm not sure if there's anything else I should have been looking for. Your changes seem to match what the dev guide says is the new pybind11 system.

Show
John Parejko added a comment - I looked over a selection of the commits (the ones you listed, and a few more). Beyond the few comments I made, I'm not sure if there's anything else I should have been looking for. Your changes seem to match what the dev guide says is the new pybind11 system.
Hide
Krzysztof Findeisen added a comment -

Thanks for the heroic review!

Show
Krzysztof Findeisen added a comment - Thanks for the heroic review!

#### People

Assignee:
Krzysztof Findeisen
Reporter:
Krzysztof Findeisen
Reviewers:
John Parejko
Watchers:
Jim Bosch, John Parejko, John Swinbank, Krzysztof Findeisen