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

Move FileDescriptor to a property in Formatter

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: daf_butler
    • Labels:
      None

      Description

      Discussing DM-20763 with John Parejko leads me to think that datastore should guarantee that Formatter instances are not reused. This would allow formatter implementations to cache information with the knowledge that they don't have to worry about the cache being wrong if the file descriptor changes. To make this more obvious I'm going to investigate changing Formatter so that FileDescriptor is a mandatory constructor argument becoming a read-only property. This will make it impossible for the formatter to be used for any other file and will mean that the argument is removed from the read and write methods.

        Attachments

          Issue Links

          relates to

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-20763 Add initial support for Gen3 Butler to obs_decam

          • Done

            Activity

            Ascending order - Click to sort in descending order
            Hide
            Permalink
            Parejkoj John Parejko added a comment - - edited

            You'll have to also update HyperSuprimeCamRawFormatter.readImage() and obs.base.FitsRawFormatterBase on this PR. I think you can just cherry-pick my commits from DM-20763.

            Show
            Parejkoj John Parejko added a comment - - edited You'll have to also update HyperSuprimeCamRawFormatter.readImage() and obs.base.FitsRawFormatterBase on this PR. I think you can just cherry-pick my commits from DM-20763 .
            Hide
            Permalink
            tjenness Tim Jenness added a comment -

            I know that I have more to do. My concern was that this is such a major change to the formatter API that I wanted you and Jim Bosch to sign off before I proceeded to make all the other packages work.

            Show
            tjenness Tim Jenness added a comment - I know that I have more to do. My concern was that this is such a major change to the formatter API that I wanted you and Jim Bosch to sign off before I proceeded to make all the other packages work.
            Hide
            Permalink
            tjenness Tim Jenness added a comment -

            Given that Jim Bosch has signed off on the PR in daf_butler I'll now work on getting obs_subaru etc fixed up.

            Show
            tjenness Tim Jenness added a comment - Given that Jim Bosch has signed off on the PR in daf_butler I'll now work on getting obs_subaru etc fixed up.
            Hide
            Permalink
            tjenness Tim Jenness added a comment -

            I've cleaned this up and I think it does look better. At Jim Bosch's request I have allowed FormatterFactory to retrieve the class as well as an instance and that has made things more explicit. I think we should probably change Instrument.getRawFormatter() to return a class rather than an instance now but I think that can be done as part of DM-20763.

            Show
            tjenness Tim Jenness added a comment - I've cleaned this up and I think it does look better. At Jim Bosch 's request I have allowed FormatterFactory to retrieve the class as well as an instance and that has made things more explicit. I think we should probably change Instrument.getRawFormatter() to return a class rather than an instance now but I think that can be done as part of DM-20763 .
            Hide
            Permalink
            jbosch Jim Bosch added a comment -

            Everything looks good to me.  Just a few extremely minor comments on the daf_butler PR.  Leaving "In Review" only because there are two of us.

             

            Show
            jbosch Jim Bosch added a comment - Everything looks good to me.  Just a few extremely minor comments on the daf_butler PR.  Leaving "In Review" only because there are two of us.  
            Hide
            Permalink
            Parejkoj John Parejko added a comment -

            A couple comments on the PRs, but this looks good. Thanks for doing it.

            Show
            Parejkoj John Parejko added a comment - A couple comments on the PRs, but this looks good. Thanks for doing it.

              People

              Assignee:
              tjenness Tim Jenness
              Reporter:
              tjenness Tim Jenness
              Reviewers:
              Jim Bosch, John Parejko
              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.