[PIPE2D-766] reduceExposure assumes that all visits have the same pfsConfig file Created: 08/Mar/21 Updated: 12/Mar/21 Resolved: 12/Mar/21 |
|
| Status: | Done |
| Project: | DRP 2-D Pipeline |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Story | Priority: | Normal |
| Reporter: | rhl | Assignee: | price |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Sprint: | 2DDRP-2021 A3 |
| Reviewers: | hassan |
| Description |
|
It is not going to be correct to assume that all visits passed to reduceExposure have the pfsConfig file. Please fix the code to handle this (the worst and simplest solution would be to check when the assumption is violated, but I think we should do better than that). |
| Comments |
| Comment by price [ 09/Mar/21 ] |
|
reduceExposure is operating over all arms from all spectrographs of a single visit. These absolutely should all have the same pfsConfig. |
| Comment by rhl [ 10/Mar/21 ] |
|
You can give it a list of visits, and it helpfully processes all of them. If this is not the intended behaviour we should discuss, and if we decide to stick with it add appropriate checks. I'd rather fix the pfsConfig assumption. |
| Comment by price [ 10/Mar/21 ] |
|
I don't think it's an assumption. There's only one pfsConfig for a visit, and so all arms for that visit have the same pfsConfig. The TaskRunner ensures that it receives one visit at a time. Or are you concerned about running it from a python prompt, operating on what it's given in violation of the (admittedly not crystal clear) docs? |
| Comment by rhl [ 12/Mar/21 ] |
|
This one sounds like user error that we can close as mistaken (or do we have to use "won't fix"?). Running from a notebook is my problem, although I'd probably add a check that all the data being processed is indeed from the same visit. |
| Comment by price [ 12/Mar/21 ] |
|
Added a check that all visits are the same. |
| Comment by hassan [ 12/Mar/21 ] |
|
Approved with no further changes requested. |
| Comment by price [ 12/Mar/21 ] |
|
Merged to master. |