Details
-
Type:
RFC
-
Status: Implemented
-
Resolution: Done
-
Component/s: DM
-
Labels:
Description
SourceDetectionTask.run() in meas_algorithms includes an fpSets item in the return struct, which is just a copy of the result from detectFootprints(). This was added during a refactor by Paul Price in 2018, and has a # Backward compatibility comment on it. I've seen confusion about the contents of the SourceDetectionTask return structure (partly caused by it not being fully documented, which I'm fixing on a user-branch ticket); the fpSets item is completely redundant, and people should just use the positive/negative/etc. items of the run return struct instead.
We don't have a good way to deprecate return values from functions and Tasks, so I am proposing to just remove this item from the return struct, and fix our internal usage of it. I don't know of a better approach for dealing with external users and return struct contents, but we have some other places where we should remove things from a return struct (e.g. DM-35940), and no good way to manage that deprecation.
Attachments
Issue Links
Activity
CCB agrees that in this case we can get by without a deprecation, but in the future it would be good to have functionality (preferably in Struct itself) to allow for deprecation, as it's unlikely future removals would be as innocuous as this one.
Personally, I'm also attracted to the idea of using method-specific dataclasses instead of Struct, not just for deprecations, but as a general preference - but I don't feel strongly enough to try to change a well-established pattern right now, even for new code (if someone else does, and wants to make another RFC, you'll probably get my vote).
Adopting with implementation ticket DM-37468.
John Parejko are you ready to mark this RFC implemented? The triggered ticket has been completed.
In this case, a drastic but common mechanism would be to create a new method, perhaps with a versioned name, that has a different effective return type (not in the mypy sense). Unfortunately, run() is built into our PipelineTask interface definition, so that mechanism is not available.
The only thing I can think of is to return either a class instance specific to this method or a more-generic DeprecatingStruct that behaves like the usual lsst.pipe.base.Struct but that warns on access to the fpSets component (or other specified component(s)).