extra keyword arguments to find_bad_channels_maxwell, notch_filter, and filter_data; bugfix compute_rank channel types

[harrisonritz]

Sorry there are so many commits, I had copied this branch from the branch I did the other PR on (lessons learned).

• new config options that let you provide a dictionary of keyword argument to find_bad_channels_maxwell, notch_filter, and filter_data.
• now allow for notch_filter to have a frequency of None, since this is allow for method="spectrum_fit"
• bugfix for compute_rank that allows for mag sensors, rather than meg sensors (sorry this is tangential). This was breaking for OPM, which only has mag.
• new test for custom keyword arguments

I haven't added any checking for whether these keyword arguments overlap with existing arguments, because the MNE functions already will throw an error if you try to give them multiple values.

Before merging …

• Changelog has been updated (docs/source/dev.md.inc)

[hoechenberger]

Great work, I left a few comments

[hoechenberger]

LGTM but I haven't actually looked at the test cases

[larsoner]

A couple of things here:

1. ds000248 is not our fastest dataset to run tests for. For this and other similar extensions, it's best if possible to change some existing config if possible. If not possible (which should be fairly rare), then we should add one that executes as quickly as possible.

2. In the future, when we do want to add a new test config (since I don't think we need to here), changes need to be made to .circleci/config.yml to get it to actually run. You can see that the new config here wasn't run:

   

With this in mind, I think in a previous PR you also added another config, is there a dataset that runs quicker that you could add a config for instead of ds000248? You can see the timings here

https://app.circleci.com/pipelines/github/mne-tools/mne-bids-pipeline/4974/workflows/c3e97eed-cc21-4945-b826-c6650ad8f9b8

and you can cross-ref with the ones listed as being MEG but not using maxfilter here:

https://mne.tools/mne-bids-pipeline/stable/examples/examples.html#demonstrated-features

[harrisonritz]

this is super helpful, thanks. makes sense the that goal is coverage not isolating issues. revising this now, but running into weird issues in my local install (below).

[harrisonritz]

I put the tests on ds004229

[harrisonritz]

@larsoner when I run my tests locally, this line doesn't work:

mne-bids-pipeline/mne_bids_pipeline/_config_utils.py

Lines 458 to 464 in 30c7798

	bids_path = BIDSPath(
	subject=subject,
	session=session,
	suffix="meg",
	datatype="meg",
	root=config.bids_root,
	).match()

For ds004229, the path I get is: ~/mne_data/ds004229/sub-102/meg/sub-102_meg, which doesn't match anything.
The correct path is ~/mne_data/ds004229/sub-102/meg/sub-102_acq-crosstalk_meg.fif.

The older method works fine:

BIDSPath(subject=subject,
            session=session,
            suffix="meg",
            datatype="meg",
            root=config.bids_root,
        ).meg_calibration_fpath

But -- this should mean that more of the tests should be failing, because the default for failing to find this path is to raise an error.
So I must have something weird about my install, eh? I should have the latest mne_bids (0.17.0.dev28+g7b046947e).

[harrisonritz]

It must just be something funny in my install; I'll reinstall my environment. Maybe worth having an environment/requirement spec.

[hoechenberger]

Maybe worth having an environment/requirement spec.

Once we switch to uv, we will provide a lock file.

[harrisonritz]

uv looks cool