movement
movement copied to clipboard
neuroinformatics-unit
Make `report_nan_stats` and `calculate_nan_stats` more permissive about dimensions
Originally reported by @stellaprins in #429:
I tried using the :func:movement.filtering.median_filter function on the pupil size to replace my moving average filter (usingnp.convolve with mode="same") with the moving median filter used in movement but it wants space as one of the array dimensions
My reply was:
That's not a bad idea at all, it should work. I think the problem arises because of the report_nan_values function, which in turn calls calculate_nan_stats, which contains a reference to the space dimension. If I'm right, calling the median filter with print_report=False should work. Assuming we confirm my theory, this is a bug that should be fixed.
In general, we should re-examine how report_nan_stats and calculate_nan_stats treat dimensions. They should be maximally flexible and be able to handle the absence of dimensions like space or individuals. The filters technically only require the time dimension (because they operate along that axis), so we shouldn't require any additional dimensions to run them, and definitely not because we want to print a report.
This issue will be somewhat ameliorated by addressing #343 but is essentially a separate problem.
@niksirbi Hi, I’d like to work on this issue. Could you assign it to me? Thanks!
@niksirbi Hi, I’d also like to work on this issue. Could you please assign it to me? ThanksYou!
Hey @Spkap and @Vinay-K-Rajith. There is no need to be assigned to an issue to start working on it. Instead we encourage contributors to open a draft pull request early in the process, so that others can see the approach you are taking. Just make sure to mention this issue in the pull request (e.g. "closes #issue-number")
Since both of you have expressed interest in this, feel free to coordinate and help each other with the process. This is not a requirement of course, but two brains are often better than one.
