Hi Ian, You're welcome to get started reviewing this.  The unit test for DCR is failing, but I THINK I just did something stupid with the _init_()'s with the multiple inheritance. I don't have time to look into that until later today. 

Branches on pipe_base, pipe_tasks.

I've been informed that pipe_supertask is obsolete, so don't look in there

OK, So the problem is in test_dcrAssembleCoadd. The config is being passed as a positional arg rather than a keyword arg:

        self.config = DcrAssembleCoaddConfig()

        self.task = DcrAssembleCoaddTask(self.config) 

This Gen3 PipelineTask ecosystem assumes Task's have no positional arguments. Your unit test isn't standard usage, but you're surely not the only one who has passed a config in like this. I bet there are notebooks out there that do it too, so I consider this (PipelineTask conversion) an API change. I'm deciding on how to best handle this. Catch any args not in the kwargs, put them in the kwargs, and raise a Deprecation warning?

I'm really grateful for your unittest, because someone someday would be surprised about why their config wasn't being respected.

I'm glad you figured it out, because I was having no luck with it, and was looking at all the wrong things. Yes, my two dcr-related unit tests do pass if I change that line to self.task = DcrAssembleCoaddTask(config=self.config). That might be the quick way forward for now, along with filing a new ticket (and RFC?) to deal with the more significant API change.

Looks good. I have tested dcrAssembleCoaddTask with these changes, and it appears to work as expected. I had mostly only requests for additional docstrings and code comments, which are hopefully easy for you to add.

I have the one-line change to test_dcrAssembleCoadd.py on my machine that does the easy fix to make the tests pass. I can commit and push that if you like. I suggest you put off the harder changes of dealing with positional arguments like I had in that unit test for later work, but please do make the ticket for that now.

Thank you for the review!! No need to push the one-liner in the unit test (I have it locally).

About to merge. Note: This ticket changes the logging output for tests/nopytest_test_coadds.py. master_output.log DM17045_output.log I'm attaching the before and after for posterity. Snippet from the after:

compareWarpAssembleCoadd INFO: Found 7 deepCoadd_directWarp

compareWarpAssembleCoadd.assembleStaticSkyModel INFO: Weight of deepCoadd_psfMatchedWarp DataId(initialdata={'visit': 2, 'filter': 'r', 'patch': '0,0', 'tract': 0}, tag=set()) = 321.155

compareWarpAssembleCoadd.assembleStaticSkyModel INFO: Weight of deepCoadd_psfMatchedWarp DataId(initialdata={'visit': 3, 'filter': 'r', 'patch': '0,0', 'tract': 0}, tag=set()) = 122.835

compareWarpAssembleCoadd.assembleStaticSkyModel INFO: Weight of deepCoadd_psfMatchedWarp DataId(initialdata={'visit': 4, 'filter': 'r', 'patch': '0,0', 'tract': 0}, tag=set()) = 31.457

compareWarpAssembleCoadd.assembleStaticSkyModel INFO: Weight of deepCoadd_psfMatchedWarp DataId(initialdata={'visit': 6, 'filter': 'r', 'patch': '0,0', 'tract': 0}, tag=set()) = 50.734

compareWarpAssembleCoadd.assembleStaticSkyModel INFO: Weight of deepCoadd_psfMatchedWarp DataId(initialdata={'visit': 7, 'filter': 'r', 'patch': '0,0', 'tract': 0}, tag=set()) = 38.547

compareWarpAssembleCoadd.assembleStaticSkyModel INFO: Weight of deepCoadd_psfMatchedWarp DataId(initialdata={'visit': 10, 'filter': 'r', 'patch': '0,0', 'tract': 0}, tag=set()) = 384.201

compareWarpAssembleCoadd.assembleStaticSkyModel INFO: Weight of deepCoadd_psfMatchedWarp DataId(initialdata={'visit': 12, 'filter': 'r', 'patch': '0,0', 'tract': 0}, tag=set()) = 10.307

Note it's not logging that it can't find the visits that don't cover the patch (e.g. 1, 5 ..)  anymore. This is because CompareWarp now passes the list of WarpRefs to its subtask's runDataRef rather than the original user-supplied list of CalexpRefs. They don't need to be grouped into visits and checked again, and it only tries to fetch the visits in the WarpRefList (and we've already confirmed the direct warp overlaps). 

See https://github.com/lsst-dm/demo_hsc_pipelinetask for configs to run this on ci_hsc

pipetask  -b /project/yusra/ci_hsc/DATA/butler.yaml -i test/ci_hsc -d "Tract.tract=0 and Patch.patch=69" -o test/<outputCollection>  run -t assembleCoadd.CompareWarpAssembleCoaddTask --configfile CompareWarpAssembleCoaddTask:compareWarpAssembleCoadd.py