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

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

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: afw
    • Labels:
      None

      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 <module>
          from ._typehandling import Storable
      python/lsst/afw/table/io/__init__.py:23: in <module>
          from .persistable import *
      python/lsst/afw/fits/__init__.py:2: in <module>
          from .pickleFits import reduceToFits, unreduceFromFits
      python/lsst/afw/fits/pickleFits.py:4: in <module>
          import lsst.afw.image
      python/lsst/afw/image/__init__.py:25: in <module>
          from .apCorrMap import *
      python/lsst/afw/image/apCorrMap/__init__.py:23: in <module>
          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 <module>
          from ._typehandling import Storable
      python/lsst/afw/table/__init__.py:28: in <module>
          from .aggregates import *
      python/lsst/afw/table/aggregates/__init__.py:23: in <module>
          from .aggregates import *
      python/lsst/afw/geom/__init__.py:30: in <module>
          from .polygon import *
      python/lsst/afw/geom/polygon/__init__.py:25: in <module>
          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.

        Attachments

          Issue Links

            Activity

            Hide
            krzys 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
            krzys 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
            krzys 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
            krzys 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
            Parejkoj 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
            Parejkoj 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
            krzys Krzysztof Findeisen added a comment -

            Thanks for the heroic review!

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

              People

              Assignee:
              krzys Krzysztof Findeisen
              Reporter:
              krzys Krzysztof Findeisen
              Reviewers:
              John Parejko
              Watchers:
              Jim Bosch, John Parejko, John Swinbank, Krzysztof Findeisen
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.