Details
-
Type:
Story
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: obs_lsstSim
-
Labels:
-
Story Points:2
-
Epic Link:
-
Sprint:Alert Production F16 - 10, Alert Production F16 - 11
-
Team:Alert Production
Description
The eimage dataset type is special to obs_lsstSim so was not updated in the process of implementing the VisitInfo ticket.
Attachments
Activity
| Field | Original Value | New Value |
|---|---|---|
| Epic Link |
|
| Status | To Do [ 10001 ] | In Progress [ 3 ] |
| Reviewers | John Parejko [ parejkoj ] | |
| Status | In Progress [ 3 ] | In Review [ 10004 ] |
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.
| Status | In Review [ 10004 ] | Reviewed [ 10101 ] |
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?
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).
I added a unit test. Can you have a look? The jenkins build passes on py2 (#17666).
| Status | Reviewed [ 10101 ] | In Review [ 10004 ] |
A few remaining comments, but otherwise looks good to merge.
| Status | In Review [ 10004 ] | Reviewed [ 10101 ] |
| Sprint | Alert Production F16 - 10 [ 284 ] | Alert Production F16 - 10, Alert Production F16 - 11 [ 284, 289 ] |
| Resolution | Done [ 10000 ] | |
| Status | Reviewed [ 10101 ] | Done [ 10002 ] |
Merging to master with passing Jenkins.
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.