Skip to content

Consider revising the Combiner machinery into a class heirarchy #578

Description

@eteq

I was trying to use Combiner recently, and I found it a bit awkward to use because a single combiner class has all the methods. So I want to through out an idea for a new way to lay out the combiner:

  • Have a CombinerBase or something like that which implements most of the bookeeping - pretty much everything except the *_combine methods. Include a combine_images abstract method.
  • Have separate MedianCombiner, SumCombiner, and AverageCombiner classes that basically just implement combine_images, using parameters from the init function, not method keywords.
  • Leave the existing Combiner, but it basically is just a compatibility class that wraps the above (might be deprecated in the future, but could also be left in with just a pointer that the class heirarchy is the "new" way).

My reasoning behind all of this is that it makes it much easier to use these classes in a pipeline, because in this scheme, you can create a *Combiner and then just call combine_images, and a downstream user can swap in various combiner types without modifying their pipeline. It also makes implementing more complex uncertainty propogation schemes a la #569 much easier

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions