DM-43878: Supporting multiple plotted quantities in CalibAmpScatterTool#265
DM-43878: Supporting multiple plotted quantities in CalibAmpScatterTool#265weatherhead99 wants to merge 1 commit intomainfrom
Conversation
weatherhead99
force-pushed
the
tickets/DM-43878
branch
from
3d50402 to
6593043
Compare
This commit changes the `quantityKey` field in the `CalibAmpScatterTool` class to be of type `ListField`. This enables multiple quantities to be plotted on a single scatter plot. In addition, pipeline YAML files for calibration verification are re-worded to use the new `ListField` type
weatherhead99
force-pushed
the
tickets/DM-43878
branch
from
b57cba3 to
9078f61
Compare
|
this now works and I've verified that the plots produced are identical to the original. Should we wish to change some plots to have multiple series in the calibAmpScatterTool it can now be done with the new ListField for quantityKey |
|
Can I ask that this be renamed according to the style guide (https://developer.lsst.io/work/flow.html#make-a-pull-request)? I'm also somewhat hopeful that this ticket will magically fix another problem I'm having with these plot types, so I'm sorry it's taken so long to get to the review. |
| doc="Figure size.", | ||
| default=[8, 8], | ||
| ) | ||
| figsize = ListField[float](doc="Figure size.", default=[8, 8], length=2) |
There was a problem hiding this comment.
Can this be expanded out to match the other fields formatting?
There was a problem hiding this comment.
@czwa I'm not sure I understand, do you just mean the code formatting? Or to change the other fields to further constrain their formatting?
There was a problem hiding this comment.
Just code formatting, and I prefer the expanded "one element per line" version.
weatherhead99
changed the title
|
I think this has been open long enough, so ping me after you've rebased, I'll do one final quick scan, and we can move ahead with getting this merged (without waiting for @natelust ). |
2 participants
This is roughly how I'm thinking to implement the ability to plot multiple data series in CalibAmpScatterTool, and how the updated pipelines would look. It doesn't quite work yet, a couple more commits to come.