Completed the refactoring and renaming of classes. Did not rename any of the python modules (forcedImage, forcedCcd, or forcedCoadd) even though I don't like any of them. We can change those now or later.

On tickets/DM-241
meas_base:

bin/forcedCcd.py | 4 +-
bin/forcedCoadd.py | 4 +-
python/lsst/meas/base/forcedCcd.py | 18 ++--
python/lsst/meas/base/forcedCoadd.py | 12 +--
python/lsst/meas/base/forcedImage.py | 175 +++++++++++++++++++---------------
tests/testForced.py | 34 +++----
6 files changed, 130 insertions, 117 deletions

obs_lsstSim:

policy/LsstSimMapper.paf | 64 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 64 insertions

Please review these 8 new datasets. I'm not really sure they are in the right place.

Review complete. A few minor comments, and one moderate one:

• Docstring at forcedImage.py:242 about ForcedMeasurementTask needing to be subclassed for CCDs and Coadds is now obsolete.

• Also in ForcedMeasurementTask: you should always use **kwds in a Task's __init__, and pass those to Task.__init__. Leave it to that to handle the config, parentTask, and name arguments. In the TestForcedMeasurementTask, I think you can just remove the __init__ method entirely (or maybe you could just remove the class?)

• The "expregion" and "newsource" variables in generateSources() should be "expRegion" and "newSources", respectively, to comply with LSST code standards.

• What do you think about moving generateSources() to ProcessImageForcedTask? ProcessImageForcedTask.run() could then call generateSources(), and then pass the SourceCatalog to be filled to ForcedMeasurementTask. This would make ForcedMeasurementTask a little more similar to SingleFrameMeasurementTask, and then generateSources() could then call makeIdFactory() itself.

Also, new datasets in obs_lsstSim look fine. I don't think there really is much of a "right place" for them within the file.

I don't really understand how the kwds convention works, I am afraid. I added the extra arguments to _init_ just to satisfy the pipe_base code which I never understood. Perhaps at some point you can explain what sets the requirements for the input parameters on both Task and CmdTask.

You last comment is actually the refactor struggle from last week which caused me to not be done. I originally wanted generateSources to be higher up to make the idFactory need easier to satisfy. When I did it that way, it also meant that the mapper.getOutputSchema() needs to we available to ProcessImageForcedTask prior to run. I didn't much like that, but I suppose that I can have a method like ForcedMeasurementTask.getOutputSchema() which could be called prior to init. I guess we should have finished chatting this issue out last week.

Please send me your opinion before 9:00 tonight (Sunday) and I can try to implement this evening.

The kwds are just a way of forwarding function arguments from one call to another. In this case, the extra task arguments that need to be passed to the base class must be present because they're passed by the makeSubtask() and parseAndRun() methods, and the base take constructor needs them to work properly.

I'd be OK with resolving the issue with moving generateSources() by just having it refer to self.measurement.mapper.getOutputSchema(), or by having ForcedMeasurementTask set self.schema to that in its ctor and having generateSources() refer to self.measurement.schema.

Personally, I think that a design where the subtask is in charge of knowing about the source schema is a better design. On the theory that the calling class doesn't need to have this information, but the subtask does.

However, I'm all for consistency, since measurement.py and sfm.py are already built supplying the schema and measCat.

I think I would prefer to do it the way it is done in sfm, using a writable schema variable.

I think that's a fair characterization: the way we're doing this in all the process tasks is a little ugly, because the parent tasks always need to know the schema that's defined mostly by child tasks, but it'd take a huge amount of work to change that across the board, and it's better to maintain consistency for now.

Accident