Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-802

Add analysis_drp to lsst_distrib

    XMLWordPrintable

Details

    • RFC
    • Status: Implemented
    • Resolution: Done
    • DM
    • None

    Description

      analysis_drp is a gen 3 only plotting package for QA purposes. It will hopefully replace pipe_analysis in serving DM's QA plotting needs.

      Attachments

        1. colour_colour.png
          colour_colour.png
          248 kB
        2. scatter.png
          scatter.png
          125 kB
        3. skyPlot.png
          skyPlot.png
          270 kB

        Issue Links

          Activity

            jbosch Jim Bosch added a comment - - edited

            Big from me.  pipe_analysis was never added to lsst_distrib in large part because we never got it conformant with DM's coding standards and workflow, while analysis_drp was designed to be conformant from the beginning.  We want to start running some of its PipelineTasks in our production pipelines, including the ones for DP0.2.

            jbosch Jim Bosch added a comment - - edited Big from me.  pipe_analysis was never added to lsst_distrib in large part because we never got it conformant with DM's coding standards and workflow, while analysis_drp was designed to be conformant from the beginning.  We want to start running some of its PipelineTasks in our production pipelines, including the ones for DP0.2.
            sophiereed Sophie Reed added a comment -

            Included are some examples of the plots that code from analysis_drp makes. There are more plot types being developed.

            sophiereed Sophie Reed added a comment - Included are some examples of the plots that code from analysis_drp makes. There are more plot types being developed.
            lskelvin Lee Kelvin added a comment -

            I fully support this RFC. As someone who has been using analysis_drp for a while now, and beginning to develop plots of my own using analysis_drp, I find this resource to be incredibly valuable.

            lskelvin Lee Kelvin added a comment - I fully support this RFC. As someone who has been using analysis_drp for a while now, and beginning to develop plots of my own using analysis_drp , I find this resource to be incredibly valuable.
            ktl Kian-Tat Lim added a comment - - edited

            The table file should have astropy, matplotlib, numpy, and scipy removed.  If this is Gen3 only, how come there is still a dependency on daf_persistence?  Should there be a direct dependency on daf_butler (rather than only indirect through pipe_base)? Can't qa_explorer be removed, as I don't see any use of its code?

            ktl Kian-Tat Lim added a comment - - edited The table file should have astropy , matplotlib , numpy , and scipy removed.  If this is Gen3 only, how come there is still a dependency on daf_persistence ?  Should there be a direct dependency on daf_butler (rather than only indirect through pipe_base )? Can't qa_explorer be removed, as I don't see any use of its code?
            Parejkoj John Parejko added a comment -

            You should probably remove the examples path: I don't think that serves a purpose in this package, most of our older examples have bitrotted because they aren't run, and the folder is empty here anyway. Similarly for bin.src, since it's empty, too.

            I'll note that the tests directory is also empty: I realize that this is mostly plotting code, but there is also some analytic code that really should have tests.

            Parejkoj John Parejko added a comment - You should probably remove the examples path: I don't think that serves a purpose in this package, most of our older examples have bitrotted because they aren't run, and the folder is empty here anyway. Similarly for bin.src , since it's empty, too. I'll note that the tests directory is also empty: I realize that this is mostly plotting code, but there is also some analytic code that really should have tests.

            Yay! Please ensure the package has some docs and tests, and no extraneous directories or table file entries, as K-T and John point out. Looking forward to borrowing lots of what you've done for analysis_ap, so the more good life choices you can make here, the happier we will all be!

            mrawls Meredith Rawls added a comment - Yay! Please ensure the package has some docs and tests, and no extraneous directories or table file entries, as K-T and John point out. Looking forward to borrowing lots of what you've done for analysis_ap, so the more good life choices you can make here, the happier we will all be!
            Parejkoj John Parejko added a comment -

            Meredith's comment suggests to me that we might want an `analysis_base` class, if the drp and ap codes are going to be sharing things (I don't know if they will, but it's worth considering).

            Parejkoj John Parejko added a comment - Meredith's comment suggests to me that we might want an `analysis_base` class, if the drp and ap codes are going to be sharing things (I don't know if they will, but it's worth considering).
            jbosch Jim Bosch added a comment -

            Meredith's comment suggests to me that we might want an `analysis_base` class, if the drp and ap codes are going to be sharing things

            Yup, this was at least vaguely part of the plan all along - we just decided to stand up something that worked before trying to figure out which bits we might want to factor out into a more general package.  I'd like to keep that separate from this RFC, but I think it might make sense to do either while standing up a corresponding AP package or just after (with either some duplication or a AP->DRP dependency as we work through the refactoring).

            I'd be interested to hear sophiereed 's thoughts on whether the boundary between DRP-specific and more general stuff has ended up pretty clear already in the current version, though.

            jbosch Jim Bosch added a comment - Meredith's comment suggests to me that we might want an `analysis_base` class, if the drp and ap codes are going to be sharing things Yup, this was at least vaguely part of the plan all along - we just decided to stand up something that worked before trying to figure out which bits we might want to factor out into a more general package.  I'd like to keep that separate from this RFC, but I think it might make sense to do either while standing up a corresponding AP package or just after (with either some duplication or a AP->DRP dependency as we work through the refactoring). I'd be interested to hear sophiereed 's thoughts on whether the boundary between DRP-specific and more general stuff has ended up pretty clear already in the current version, though.
            tjenness Tim Jenness added a comment -

            sophiereed is there a hold up on this RFC that is preventing it from being adopted?

            tjenness Tim Jenness added a comment - sophiereed is there a hold up on this RFC that is preventing it from being adopted?
            sophiereed Sophie Reed added a comment - Implementation ticket:  https://jira.lsstcorp.org/browse/DM-32711

            People

              sophiereed Sophie Reed
              sophiereed Sophie Reed
              Colin Slater, Jim Bosch, John Parejko, Kian-Tat Lim, Lee Kelvin, Meredith Rawls, Sophie Reed, Tim Jenness, Yusra AlSayyad
              Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:
                Planned End:

                Jenkins

                  No builds found.