Details
-
Type:
Story
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: None
-
Labels:None
-
Story Points:8
-
Epic Link:
-
Sprint:AP S19-2, AP S19-3, AP S19-4, AP S19-5
-
Team:Alert Production
Description
Where possible, add a (deprecated, following the procedure in RFC-557 unless that RFC has been rejected by the time you get to this) backwards compatible API to PhotoCalib to enable it to act as a drop-in replacement for Calib.
Per discussion of 2018-12-19, the most important goal here is to provide conversion to/from magnitudes using the same API as Calib uses, but, in general, the aim of this ticket is to minimize avoidable disruption to “science users” when PhotoCalib replaces Calib, without imposting an unmanageable development load. (To meet the latter goal, I suggest timeboxing initial work on this to no more than a week or so before sending for review.)
Attachments
Issue Links
Activity
| Field | Original Value | New Value |
|---|---|---|
| Epic Link |
|
| Sprint | AP S19-2 [ 830 ] | AP S19-2, AP S19-3 [ 830, 831 ] |
| Sprint | AP S19-2, AP S19-3 [ 830, 831 ] | AP S19-2, AP S19-3, AP S19-4 [ 830, 831, 832 ] |
| Status | To Do [ 10001 ] | In Progress [ 3 ] |
| Watchers | Jim Bosch, John Parejko, John Swinbank [ Jim Bosch, John Parejko, John Swinbank ] | Jim Bosch, John Parejko, John Swinbank, Yusra AlSayyad [ Jim Bosch, John Parejko, John Swinbank, Yusra AlSayyad ] |
Yet another two part review.
Russell Owen: Can you please review the code (~175 lines, mostly C++)? It does not yet have python-level deprecation warnings, because deprecated is not yet available to the stack.
Lauren MacArthur: can you please test out the u/parejkoj/testCalibDeprecation branch (where I rebased this on top of DM-10156), with the rest of a DM-10156 checkout (I think Yusra AlSayyad made one on lsst-dev?), to see if I covered enough ground with the deprecated API? Yusra AlSayyad suggested you might have some code that would exercise it (I should have already caught everything in lsst_distrib that relied on Calib).
| Reviewers | Lauren MacArthur, Russell Owen [ lauren, rowen ] | |
| Status | In Progress [ 3 ] | In Review [ 10004 ] |
Lauren MacArthur is out this week. I recommend Sophie Reed to test out the branch on lsst-dm/pipe_analysis.
| Reviewers | Lauren MacArthur, Russell Owen [ lauren, rowen ] | Russell Owen, Sophie Reed [ rowen, sophiereed ] |
| Reviewers | Russell Owen, Sophie Reed [ rowen, sophiereed ] | Sophie Reed [ sophiereed ] |
These code changes look great. I posted a few small suggestions on the PR. My only substantive concern is that the new setThrowOnNegativeFlux silently ignores true.
New post review, post rebase Jenkins: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/29592/pipeline
I merged this earlier today before noticing that Sophie Reed had not signed off. However, this will probably make testing easier, since I can just rebase DM-10156 on top of this and then you can use just that ticket to do your backwards compatibility testing.
I've pushed an attempted fix for meas_mosaic (handling the refcat colorterms better): can you please test it?
| Sprint | AP S19-2, AP S19-3, AP S19-4 [ 830, 831, 832 ] | AP S19-2, AP S19-3, AP S19-4, AP S19-5 [ 830, 831, 832, 833 ] |
Merged and done
| Resolution | Done [ 10000 ] | |
| Status | In Review [ 10004 ] | Done [ 10002 ] |
Jenkins run: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/29560/pipeline