WIP FIX Deprecate use_rounding=False default in time_as_index [circle full]

[mathias-sm]

Reference issue (if any)

Implements the changes suggested in #13634 to estimate the impact of defaulting to truncation to defaulting to rounding when converting time to indices.

What does this implement/fix?

time_as_index defaulted to truncation (use_rounding=False), causing get_data(tmin=...) to return different samples from crop(tmin=...).get_data() at certain times due to floating-point truncation. Change the default to None and rounding. Emits a FutureWarning prompting callers to pass True or False explicitly. The goal of this commit is to estiate the impact of this change; follow-up commit should internally update all use of time_as_index to explicitely set a desired default.

Additional information

I have not yet updated all of the internal use of time_as_index, some of which explicitely use either use_rounding=False or use_rounding=False. Internall calls that do not pass a value for use_rounding will emit warnings. This is not ready for merging and is submitted as WIP / as a draft PR to trigger all tests and estimate the impact of this change.

[welcome]

Hello! 👋 Thanks for opening your first pull request here! ❤️ We will try to get back to you soon. 🚴

[mathias-sm]

https://dev.azure.com/mne-tools/mne-python/_build/results?buildId=36780&view=logs&j=d2de176e-a8da-5d2d-82f1-e561f5072d99&t=f4deb373-ce79-59c7-15db-2653b3e8f79f

I had not realized that so many tests would fail because of the warning and not because of resulting discrepancies in the computation of time_as_index. Given that the goal is to see the impact of the change I am tempted to remove the warning, but I could also be convinced to change tests instead?

Thanks

[mathias-sm]

I went ahead and removed the warning for the time being, so that we can see the impact without having to make changes all over the codebase. It also seems like some CircleCI runs require approval from a maintainer? I could not figure out if there was something I could do on my own. I see this is currently discussed in #13837 -> if it helps I can report that despite linking CirclCI to my github account, I do not see a way around block-unregistered-user.

[mathias-sm]

Update about CircleCI: it was not intuitive to me, but I could "log in with github account" (they hide an arrow which lets you only link public repository which I prefer), then find my mne-tools under the "projects" tab then click 'set up' and chose "Fastest: Use the .circleci/config.yml in my repo" and manually input main for the branch and click "Set Up Project". Now it seems to be running?

[mathias-sm]

As far as I can tell there is only one test failure, in mne/io/egi/tests/test_egi.py::test_egi_mff_pause, which does indeed boil down to a raw.time_as_index that relied on truncating instead of rounding.

I can see two fixes: either explicitely pass use_rounding=False which indeed makes the tests pass, or update egi_pause_skips/egi_eprime_pause_skips/egi_pause_w1337_skips with the values one get when using rounding. Interestingly, I tested the first change and it works, despite the fact that one might fear that the following assert is not true anymore:

        start, stop = raw.time_as_index(
            [annot["onset"], annot["onset"] + annot["duration"]],
            use_rounding=False
        )
        data, _ = raw[:, start:stop]
        assert_array_equal(data[other_picks], 0.0)

This suggest that there are 0s in a few more samples than what is selected with the annot. Not entirely sure what to do here?

In the meantime, to see if this is indeed the only place that relied on the truncation behavior in the script, I am going to commit the use of use_rounding=False in that specific test and push to trigger another comprehensive set of tests.