DM-44085: Generalize runQuantum to allow for arbitrary connections #242
Conversation
|
I'll fix the linting errors while rebasing and reorganizing the commits. |
| plotInfo = self.parsePlotInfo(inputs, dataId) | ||
| data = self.loadData(inputs["data"]) | ||
| skymap = None if "skymap" not in inputs.keys() else inputs["skymap"] | ||
| camera = None if "camera" not in inputs.keys() else inputs["camera"] |
There was a problem hiding this comment.
So, if I read this correctly, we no longer need a special carve-out for camera here - is that correct? I see the base runQuantum has a carve-out for skymap, but not for camera.
There was a problem hiding this comment.
In this ticket, even the base class shouldn't have a special carve-out for skymap. Defining connections should be sufficient, and no specializion of the runQuantum method is required. This is the main motivation for this ticket, because it was causing much confusion among the SciUnit members.
There was a problem hiding this comment.
Should we therefore also remove the special carve out for skymap here too?
Or is this needed elsewhere?
There was a problem hiding this comment.
The PR removes this. I made an inline comment just now.
arunkannawadi
force-pushed
the
tickets/DM-44085
branch
2 times, most recently
from
733fb97
to
5c21d4b
Compare
|
This looks good, and captures a number of good QoL improvement! 👍 |
This function is superseded by a method of the same name attached to AnalysisPipelineTask.
The signature of the `run` method is generic enough to take arbitrary keyword arguments, and the caller `runQuantum` should pass all the relevant inputs without placing unnecessary restrictions.
parsePlotInfo in DiffimDetectorVisitSpatiallySampledPlotsTask. This avoids an unnecessary override of `runQuantum` and the relevant logic is implemented in `parsePlotInfo` instead of in `runQuantum`.
This a) sets `rcParams` once while importing `analysis_tools` and b) enables setting it even when tasks that override the `runQuantum` method are run.
arunkannawadi
force-pushed
the
tickets/DM-44085
branch
from
5c21d4b
to
a9b5b23
Compare
No description provided.