Tickets/DM-35607: Get plot information from run quantum #22
Conversation
sr525
force-pushed
the
tickets/DM-35607
branch
2 times, most recently
from
05b102c to
a09b0e8
Compare
cmsaunders
left a comment
cmsaunders
left a comment
There was a problem hiding this comment.
This looks good to me. My only major comment is that it would be nice to add the plot information for the other plot types. It's your call on whether to do this, but you might want to poll people on what will be shown at the PCW, since it would be good to have the plot information on anything that is going to be shown to others.
| return base | ||
|
|
||
| def __call__(self, data: KeyedData, **kwargs) -> Mapping[str, Figure] | Figure: | ||
|
|
There was a problem hiding this comment.
I don't think you want to add a blank line here.
| vectorKey="patchWhole", selector=VectorSelector(vectorKey="starSelector") | ||
| ) | ||
|
|
||
| self.process.calculateActions.stars = ScatterPlotStatsAction( |
There was a problem hiding this comment.
You need to import ScatterPlotStatsAction. This is probably a rebase issue, as the import was removed on main recently.
| kwargs["plotInfo"] = _StandinPlotInfo() | ||
| for name, action in self.config.plots.items(): | ||
| for selector in action.prep.selectors: | ||
| try: |
There was a problem hiding this comment.
The AttributeError is for cases where selector.threshold does not exist, right?Instead of the try/except, how about
if "threshold" in selector.keys(): kwargs["plotInfo"]["SN"] = selector.threshold
| """ | ||
| if names is None: | ||
| names = self.collectInputNames() | ||
| names.add("patch") |
There was a problem hiding this comment.
Why not just put this line in collectInputNames? This would also prevent the mypy error that you are getting because not every Iterable has an add method.
There was a problem hiding this comment.
This was left over from when I was experimenting with something. I've removed it and everything seems to be working.
sr525
force-pushed
the
tickets/DM-35607
branch
2 times, most recently
from
7b6d1bb to
0230df5
Compare
I thought that it was applied to all the plot types but my test pipeline appears to have been demolished somewhere in the recent changes and the standard pipelines don't seem to have a variety of plots in them. |
sr525
force-pushed
the
tickets/DM-35607
branch
8 times, most recently
from
1aa6cba to
58eb875
Compare
No description provided.