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

Add FITS image, catalog readers that infer types from file

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: afw
    • Labels:
    • Story Points:
      2
    • Sprint:
      BG3_F18_08, BG3_F18_09
    • Team:
      Data Release Production

      Description

      As they're implemented in C++ and return non-polymorphic types, all of our FITS readers coerce the on-disk object to a specific type.  Gen3 Butler code could be greatly simplified by providing Image, Mask, Exposure, and Catalog readers that return infer the template specialization from what's actually in the file.

        Attachments

          Issue Links

            Activity

            Hide
            jbosch Jim Bosch added a comment -

            Nate Lust, hate to dump a big review like this on you right now, as I know you tend to attract them, but I have been or will be dumping other big reviews on all of the other candidates.  I'm hoping you'll find some of the modern C++ functional stuff in the pybind11 code more fun than scary, and maybe even have some ideas on how to make it less scary (I think it's actually quite readable and expressive once you've internalized C++ lambda capture syntax, but I'm well aware that's not something most readers of this code will have done).

            Show
            jbosch Jim Bosch added a comment - Nate Lust , hate to dump a big review like this on you right now, as I know you tend to attract them, but I have been or will be dumping other big reviews on all of the other candidates.  I'm hoping you'll find some of the modern C++ functional stuff in the pybind11 code more fun than scary, and maybe even have some ideas on how to make it less scary (I think it's actually quite readable and expressive once you've internalized C++ lambda capture syntax, but I'm well aware that's not something most readers of this code will have done).
            Hide
            jbosch Jim Bosch added a comment -

            Almost all changes in afw: https://github.com/lsst/afw/pull/396

            with a one-line bugfix in daf_base: https://github.com/lsst/daf_base/pull/42

            Note that the afw changes depend on DM-15836, which is also out for review.

            Full-stack Jenkins run is now in progress, so there may be small changes for other packages appearing if I discover any breakage.

            Show
            jbosch Jim Bosch added a comment - Almost all changes in afw: https://github.com/lsst/afw/pull/396 with a one-line bugfix in daf_base: https://github.com/lsst/daf_base/pull/42 Note that the afw changes depend on DM-15836 , which is also out for review. Full-stack Jenkins run is now in progress, so there may be small changes for other packages appearing if I discover any breakage.
            Hide
            nlust Nate Lust added a comment -

            Quite a few comments, but I trust that you can address them and then merge, as there were no serious algorithmic or logic comments.

            Show
            nlust Nate Lust added a comment - Quite a few comments, but I trust that you can address them and then merge, as there were no serious algorithmic or logic comments.
            Hide
            jbosch Jim Bosch added a comment -

            Colin Slater, since Nate Lust's review of all of the afw code on this ticket, testing has revealed some (very small) fixes needed in obs_lsstSim and obs_decam.  Could you take a quick look at just those two packages, please?

            Show
            jbosch Jim Bosch added a comment - Colin Slater , since Nate Lust 's review of all of the afw code on this ticket, testing has revealed some (very small) fixes needed in obs_lsstSim and obs_decam.  Could you take a quick look at just those two packages, please?
            Hide
            ctslater Colin Slater added a comment -

            No objections here.

            Show
            ctslater Colin Slater added a comment - No objections here.

              People

              • Assignee:
                jbosch Jim Bosch
                Reporter:
                jbosch Jim Bosch
                Reviewers:
                Colin Slater
                Watchers:
                Colin Slater, Jim Bosch, Nate Lust, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel