tobac icon indicating copy to clipboard operation
tobac copied to clipboard

Need `dxy` parameter in `feature_detection_multithreshold`

Open freemansw1 opened this issue 2 years ago • 1 comments

I came across this while working on #127: the feature_detection.feature_detection_multithreshold function (which most users will be using for feature detection) is missing the dxy parameter in RC_v1.4.0:

https://github.com/tobac-project/tobac/blob/29e55984248b2a1dfb2205834dd3df0e2ad9a7a3/tobac/feature_detection.py#L535

freemansw1 avatar Oct 24 '22 16:10 freemansw1

Hm, I am slightly confused @freemansw1. When I look at feature_detection_multithreshold() in feature_detection.py for RC_v1.4.0 I can actually see the dxy parameter. Or did you mean dxy=-1 in feature_detection_multithreshold_timestep()? I can see that this one was added to your dev-3D-PBC branch after merging RC_v1.5.0 into it, because the old dev branch (which was merged into dev-3D-PBC before that ) did not contain the parameter. It looks, however, that both RC_v1.5.0 and RC_v1.4.0 are up to date and do not contain this bug. Please correct me if I am wrong!

Anyhow, this is also a good reminder to us to add a unit test for feature_detection_multithreshold(). Even if test_feature_detection_multithreshold_timestep() covers most of the code, it is probably crucial that we assure that the functions that most users will be using are properly working.

JuliaKukulies avatar Oct 24 '22 20:10 JuliaKukulies

You are totally right, @JuliaKukulies . I had completely missed it when merging in because it's at the top/is not a keyword argument. My apologies!

freemansw1 avatar Oct 25 '22 14:10 freemansw1