Add scaling to average_combine and median_combine #123
Conversation
|
Thanks for putting something together! However, I think the way that I was viewing this is that scaling would be handled the same way as rejection or as the weighting and wouldn't be a separate thing that is passed to the median or averaging tasks. So my suggestion would be actually something easier than what you have done, which would just have a self.scalings and that if it wasn't None it would be multiplied by the scalings. I could see that the scale functions could be complicated (needing sections or keywords or something else entirely in parameters passed to it), but at the same time, we didn't really define anything before hand so I could also be convinced of this method. |
|
ok, we can hash it out during the call today |
|
@crawfordsm -- looking over this again I realized I still have a question: do you see scaling as a method or as an attribute? I could see advantages either way: an attribute lets the user do anything they want, but a method maybe allows for more convenience in the case where the scaling is determined by applying the same operation to all of the images. A property would almost do the trick, so that the user could do something like: combiner.scaling = np.ma.median # apply this function to each image to determine its scaling
combiner.scaling = my_complicated_function # as needed
combiner.scaling = [1, 2, 3] # or a ndarray or anything convertible to an ndarray
combiner.scaling = None # to turn it back off, this would be the default on initialization |
|
@crawfordsm -- I updated this to make scaling a property; let me know what you think! |
| print("len", len(self._scaling)) | ||
| self._scaling = np.array(self._scaling) | ||
| print(self._scaling.shape) | ||
| print(n_images) |
There was a problem hiding this comment.
Do you still need the print statements?
There was a problem hiding this comment.
oops. Thought I caught all of those...
|
Looks good! You've left in some print statements that could be removed I think and I've added an inline comment. After that, I would be happy to have this merged. |
Add scaling to average_combine and median_combine
No description provided.