Hi @cgohlke! Thanks for implementing this improvements for median filtering. The modifications helped improving a little bit more the execution times, since now it seems to be around 20% faster than Scipy's (compared to 10-15% faster from what is now in main).

• Use separate nan_mask for real and imag to match Cython implementation.

This is something we can discuss if it makes sense to apply the same logic as the phasor_threshold for propagating NaN across real and imag so that same values are invalid in all components of the phasor. But we can keep it like this I think and remember to discuss it next meeting.

• Move median filter after thresholding in introductory tutorial. That should be better practice now that the median filter correctly handles NaN, no?

I think it would be a good idea to have the input of @lmalacrida on this. I remember he once said to me it's better practice to first filter and then threshold. I tried first thresholding and then filtering and the result phasor has wider spread and doesn't look as nice (even with the current implementation which is NaN consistent).

This is an example result of first thresholding and then filtering:

And this is first filtering and then thresholding where you can see it is more compact and fewer pixels lie outside the semicircle:

• Added a note that "filtered coordinates can no longer be used to reconstruct the original signal" (is there a better phrasing? Should that be added to the function docstring?)

This can be a good idea to add to the docstring as a warning that once performed the filter you can't go back to the signal. Also maybe add a note in the phasor_to_signal function stating the same warning?

it's better practice to first filter and then threshold

OK, I reverted the changes to the tutorial since they are disputed.

apply the same logic as the phasor_threshold for propagating NaN across real and imag so that same values are invalid in all components of the phasor.

This would need to be documented and tested. And it should be implemented for all code paths, not just one. The filter would no longer be independent for real and imag. The logic should also apply to the intensity (at least optionally). That would probably be better implemented in a separate function (?)

I think it is easier to implement and document if the median filter is applied independently (as it is now).

@bruno-pannunzio I agree that Filtering should be performed on the entire image array, regardless of thresholding or before it. This is how i believe was performed at SimFCS.

I agree that Filtering should be performed on the entire image array, regardless of thresholding or before it. This is how i believe was performed at SimFCS.

And the scientific reason is?

That would probably be better implemented in a separate function (?)

That's what phasor_threshold already does by default.

This is how i believe was performed at SimFCS.

I skimmed through the SimFCS source code. At least in some places, a minimum intensity threshold is applied as part of calculating phasor coordinates, that is before applying median filter. Phasor coordinates for intensities below threshold are set to zero, which are then included in the median filter (I presume). Other code paths in SimFCS might or might not do things differently.

For comparison: PhasorPy does not apply any intensity threshold by default when calculating phasor coordinates and sets phasor coordinates with no intensity to NaN. The phasor_threshold function sets values below or above thresholds to NaN. NaN values are ignored in the median filter.