Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make report_nan_stats and calculate_nan_stats more permissive about dimensions #453

Open
niksirbi opened this issue Feb 28, 2025 · 3 comments
Labels
enhancement New optional feature

Comments

@niksirbi
Copy link
Member

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.

@Spkap
Copy link

Spkap commented Mar 1, 2025

@niksirbi Hi, I’d like to work on this issue. Could you assign it to me? Thanks!

@Vinay-K-Rajith
Copy link

@niksirbi Hi, I’d also like to work on this issue. Could you please assign it to me? ThanksYou!

@niksirbi
Copy link
Member Author

niksirbi commented Mar 3, 2025

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New optional feature
Projects
Status: 🤔 Triage
Development

No branches or pull requests

3 participants