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

Update obs_lsstSim to add VisitInfo to eimages

    XMLWordPrintable

    Details

      Description

      The eimage dataset type is special to obs_lsstSim so was not updated in the process of implementing the VisitInfo ticket.

        Attachments

          Activity

          Hide
          krughoff Simon Krughoff added a comment -

          John Parejko will you please take a look at these changes? They are what were needed to add VisitInfo to the calexp when using processEimage.

          Show
          krughoff Simon Krughoff added a comment - John Parejko will you please take a look at these changes? They are what were needed to add VisitInfo to the calexp when using processEimage.
          Hide
          Parejkoj John Parejko added a comment -

          Reviewed. My main problem is the lack of a test. Unless this is tested by existing code (which would very much surprise me), I'd like a test for it. Of course, if there isn't a test of raw VisitInfo lsstSim stuff or if we don't have eimage files in testdata_lsstSim, then that may be hard to do.

          Show
          Parejkoj John Parejko added a comment - Reviewed. My main problem is the lack of a test. Unless this is tested by existing code (which would very much surprise me), I'd like a test for it. Of course, if there isn't a test of raw VisitInfo lsstSim stuff or if we don't have eimage files in testdata_lsstSim, then that may be hard to do.
          Hide
          krughoff Simon Krughoff added a comment -

          There is no eimage in obs_test. Is it reasonable to put a test of VisitInfo in obs_base? The problem is that we don't have a testdata_lsstSim package. What do you suggest?

          Show
          krughoff Simon Krughoff added a comment - There is no eimage in obs_test. Is it reasonable to put a test of VisitInfo in obs_base? The problem is that we don't have a testdata_lsstSim package. What do you suggest?
          Hide
          Parejkoj John Parejko added a comment -

          Per our discussion earlier, there is a data/ directory in tests where you could add an eimage to test against. Please note in the unittest that this is an lsstSim-specific test (e.g. it shouldn't get lifted into obs_base).

          Show
          Parejkoj John Parejko added a comment - Per our discussion earlier, there is a data/ directory in tests where you could add an eimage to test against. Please note in the unittest that this is an lsstSim-specific test (e.g. it shouldn't get lifted into obs_base).
          Hide
          krughoff Simon Krughoff added a comment - - edited

          I added a unit test. Can you have a look? The jenkins build passes on py2 (#17666).

          Show
          krughoff Simon Krughoff added a comment - - edited I added a unit test. Can you have a look? The jenkins build passes on py2 (#17666).
          Hide
          Parejkoj John Parejko added a comment -

          A few remaining comments, but otherwise looks good to merge.

          Show
          Parejkoj John Parejko added a comment - A few remaining comments, but otherwise looks good to merge.
          Hide
          krughoff Simon Krughoff added a comment -

          Merging to master with passing Jenkins.

          Show
          krughoff Simon Krughoff added a comment - Merging to master with passing Jenkins.

            People

            Assignee:
            krughoff Simon Krughoff
            Reporter:
            krughoff Simon Krughoff
            Reviewers:
            John Parejko
            Watchers:
            John Parejko, Simon Krughoff
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:

                Jenkins

                No builds found.