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

Generalize dataset to formatter mapping in obs packages

    XMLWordPrintable

    Details

    • Story Points:
      1.5
    • Team:
      Architecture
    • Urgent?:
      No

      Description

      In DM-22655 there was a hack left in place where cpBias and friends were listed in daf_butler as storage classes and also listed the formatters. This is not correct. Instead we need to generalize the raw->formatter mapping in obs packages to allow ingest to support arbitrary instrument specific dataset types.

        Attachments

          Issue Links

            Activity

            Hide
            tjenness Tim Jenness added a comment -

            After looking at this what I've done is add formatterClasses to the convertRepo configuration file and that formatter is then passed all the way down to TargetFileHandler where it is added to the FileDataset. Seems to work for cpBias/cpFlat.

            Show
            tjenness Tim Jenness added a comment - After looking at this what I've done is add formatterClasses to the convertRepo configuration file and that formatter is then passed all the way down to TargetFileHandler where it is added to the FileDataset. Seems to work for cpBias/cpFlat.
            Hide
            tjenness Tim Jenness added a comment -

            Jim Bosch would you mind taking a look. Most of the changes are adding a formatter parameter all the way up the call stack. It looks like I could also remove the cpBias/cpDark hard coding in BuilderTargetInput if we wanted (essentially the same changes). The main question there is whether the config file states an override FileHandler class for a dataset type name, or defines a set that indicates multi extension file handler should be used.

            Show
            tjenness Tim Jenness added a comment - Jim Bosch would you mind taking a look. Most of the changes are adding a formatter parameter all the way up the call stack. It looks like I could also remove the cpBias/cpDark hard coding in BuilderTargetInput if we wanted (essentially the same changes). The main question there is whether the config file states an override FileHandler class for a dataset type name, or defines a set that indicates multi extension file handler should be used.
            Hide
            jbosch Jim Bosch added a comment -

            Looks good!

            Show
            jbosch Jim Bosch added a comment - Looks good!
            Hide
            tjenness Tim Jenness added a comment -

            Jim Bosch did you have an opinion on the hardcoding of cpBias/cpDark for choosing the multi extension handler?

            Show
            tjenness Tim Jenness added a comment - Jim Bosch did you have an opinion on the hardcoding of cpBias/cpDark for choosing the multi extension handler?
            Hide
            jbosch Jim Bosch added a comment -

            What was done on the branches was what I expected to see, and certainly seems reasonable.  I don't think I understood what the alternative was that you were describing, but if what you've done was also what you preferred, maybe I don't have to.

            Show
            jbosch Jim Bosch added a comment - What was done on the branches was what I expected to see, and certainly seems reasonable.  I don't think I understood what the alternative was that you were describing, but if what you've done was also what you preferred, maybe I don't have to.
            Hide
            tjenness Tim Jenness added a comment -

            I haven't done anything about this line: https://github.com/lsst/obs_base/blob/master/python/lsst/obs/base/gen2to3/repoWalker/builders.py#L212

            My question is really whether we care enough for me to add similar API arguments to pass down a handler class from above (as done for the formatter) to remove this DECam-ism from obs_base. If the answer is yes then I imagine the config option should be a mapping from dataset type name to handler class.

            Show
            tjenness Tim Jenness added a comment - I haven't done anything about this line: https://github.com/lsst/obs_base/blob/master/python/lsst/obs/base/gen2to3/repoWalker/builders.py#L212 My question is really whether we care enough for me to add similar API arguments to pass down a handler class from above (as done for the formatter) to remove this DECam-ism from obs_base. If the answer is yes then I imagine the config option should be a mapping from dataset type name to handler class.
            Hide
            jbosch Jim Bosch added a comment -

            Ah, right.  Sorry I'd forgotten that that was different.  I'm okay with leaving it, just because I think this code will be irrelevant before it becomes a maintenance problem.  Fine to fix it, too, of course, and I agree the solution would be similar to what you've already done elsewhere on this ticket.

            Show
            jbosch Jim Bosch added a comment - Ah, right.  Sorry I'd forgotten that that was different.  I'm okay with leaving it, just because I think this code will be irrelevant before it becomes a maintenance problem.  Fine to fix it, too, of course, and I agree the solution would be similar to what you've already done elsewhere on this ticket.

              People

              Assignee:
              tjenness Tim Jenness
              Reporter:
              tjenness Tim Jenness
              Reviewers:
              Jim Bosch
              Watchers:
              Jim Bosch, John Parejko, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.