XMLWordPrintable

#### Details

• Type: RFC
• Status: Implemented
• Resolution: Done
• Component/s:
• Labels:
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
248 kB
2. scatter.png
125 kB
3. skyPlot.png
270 kB

#### Activity

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

Show
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.
Hide
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.

Show
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.
Hide
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.

Show
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.
Hide
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?

Show
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?
Hide
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.

Show
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.
Hide
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!

Show
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!
Hide
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).

Show
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).
Hide
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 Sophie Reed 's thoughts on whether the boundary between DRP-specific and more general stuff has ended up pretty clear already in the current version, though.

Show
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 Sophie Reed 's thoughts on whether the boundary between DRP-specific and more general stuff has ended up pretty clear already in the current version, though.
Hide
Tim Jenness added a comment -

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

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

#### People

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

#### Dates

Created:
Updated:
Resolved:
Planned End:

#### Jenkins

No builds found.