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

Provide backwards-compatibility with Calib API

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Story Points:
      8
    • 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

            Show
            Parejkoj John Parejko added a comment - - edited Jenkins run: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/29560/pipeline
            Hide
            Parejkoj John Parejko added a comment -

            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).

            Show
            Parejkoj John Parejko added a comment - 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).
            Hide
            yusra Yusra AlSayyad added a comment -

            Lauren MacArthur is out this week.  I recommend Sophie Reed to test out the branch on lsst-dm/pipe_analysis. 

            Show
            yusra Yusra AlSayyad added a comment - Lauren MacArthur is out this week.  I recommend Sophie Reed to test out the branch on lsst-dm/pipe_analysis. 
            Hide
            Parejkoj John Parejko added a comment -
            Show
            Parejkoj John Parejko added a comment - PR: https://github.com/lsst/afw/pull/443
            Hide
            rowen Russell Owen added a comment -

            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.

            Show
            rowen Russell Owen added a comment - 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.
            Show
            Parejkoj John Parejko added a comment - New post review, post rebase Jenkins: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/29592/pipeline
            Hide
            Parejkoj John Parejko added a comment -

            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.

            Show
            Parejkoj John Parejko added a comment - 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.
            Hide
            Parejkoj John Parejko added a comment -

            I've pushed an attempted fix for meas_mosaic (handling the refcat colorterms better): can you please test it?

            Show
            Parejkoj John Parejko added a comment - I've pushed an attempted fix for meas_mosaic (handling the refcat colorterms better): can you please test it?
            Hide
            Parejkoj John Parejko added a comment -

            Merged and done

            Show
            Parejkoj John Parejko added a comment - Merged and done

              People

              Assignee:
              Parejkoj John Parejko
              Reporter:
              swinbank John Swinbank
              Reviewers:
              Sophie Reed
              Watchers:
              Jim Bosch, John Parejko, John Swinbank, Lauren MacArthur, Russell Owen, Yusra AlSayyad
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.