@JD-Zhu if you look at the style run by clicking "Details" you'll see this:

mne/io/kit/coreg.py:175:89: E501 Line too long (90 > 88)
mne/io/kit/kit.py:130:19: B006 Do not use mutable data structures for argument defaults
mne/io/kit/kit.py:915:15: B006 Do not use mutable data structures for argument defaults

From CircleCI details you'll see:

RuntimeError: Error documenting RawKIT:
'kit_badcoils'

I'll comment more inline but in general when a CI fails if you scroll toward the bottom of the failed/red step it should have helpful info about what went wrong!

when all five marker coils are present in the data, sometimes there are bad coils that need to be removed post-hoc. This step needs to happen before the device-head transformation is computed during the call to read_raw_kit(). [...] The proposed solution here allows a list of bad coils to be passed in when calling read_raw_kit(), so that they can be removed once the marker coil info has been read in from the .elp and .mrk files.

Perhaps this is a naive question, but does this mean that users need to call read_raw_kit() twice? (once to load all 5 coils and notice that some are bad, and a second time to exclude the bad ones)? Would it be possible to load once, notice the bad coils, then call a new method (RawKIT.remove_marker_coils(bads=[...]) or so) that would re-compute the dev_head_t in-place? If so, that seems like a preferable / more intuitive user experience to me, and also avoids doing the I/O twice (which is usually slow).

@larsoner Thanks so much for the detailed comments and suggested changes! Is there a way to accept all the suggestions ("Add suggestion to batch" is grayed out) or should I just click "Commit suggestion" for each one? Thanks.

when all five marker coils are present in the data, sometimes there are bad coils that need to be removed post-hoc. This step needs to happen before the device-head transformation is computed during the call to read_raw_kit(). [...] The proposed solution here allows a list of bad coils to be passed in when calling read_raw_kit(), so that they can be removed once the marker coil info has been read in from the .elp and .mrk files.

Perhaps this is a naive question, but does this mean that users need to call read_raw_kit() twice? (once to load all 5 coils and notice that some are bad, and a second time to exclude the bad ones)? Would it be possible to load once, notice the bad coils, then call a new method (RawKIT.remove_marker_coils(bads=[...]) or so) that would re-compute the dev_head_t in-place? If so, that seems like a preferable / more intuitive user experience to me, and also avoids doing the I/O twice (which is usually slow).

Thanks for the comment. In this solution, users only need to call read_raw_kit() once. The bad coils are usually known beforehand (most likely written down during data acquisition), so they can be passed into read_raw_kit() straight away. That said, I think having an option to remove bad coils and re-compute the dev_head_t (as well as updating 'dig' and 'hpi_results'?) for raw data that have already been read in (as per your suggestion) is also not a bad idea. This can also be quite useful on epochs/evoked data, e.g. if sensor-space analysis has been carried out previously (without worrying about bad coils as it probably didn't matter for sensor space), it would be nice to be able to load the previous results (e.g. -epo.fif) and then apply bad marker coil removal to obtain the correct transformations for source analysis.

Edit: when I said "post-hoc" in the original post, I meant after data acquisition (rather than after reading the data in). Sorry for the confusion!

Is there a way to accept all the suggestions ("Add suggestion to batch" is grayed out) or should I just click "Commit suggestion" for each one? Thanks.

On the "Files changed" tab of this page (which links here) yes you can "add to batch" then commit multiple changes at once. But at least a couple of my suggestions were more ideas that you'd have to push changes for anyway (or one suggestion that applies in multiple places in principle) so probably easier just to push a commit!

Looks like a good start! I think a simple little test would be to look through our existing MNE-Python KIT test datasets to see if one of them has enough coils where you could remove one or two of the markers using bad_coils and ensure the dev_head_t that gets computed is similar (but not identical!). Let me know if anything is unclear or you want help with any of this!

Sounds like a good idea! Will try to test this once I can figure out the virtual environments (which always seem to get me in trouble).
Alternatively, is there a way to call functions in the dev version I'm working on rather than the stable version I have installed (e.g. maybe specify the path when importing?)

Alternatively, is there a way to call functions in the dev version I'm working on rather than the stable version I have installed (e.g. maybe specify the path when importing?)

In theory if you are in the mne-python/ directory, having mne-python/mne/ directly beneath it should make it so that import mne imports from that directory. Looks like that's the case:

~$ pip uninstall -q mne
Proceed (Y/n)? y
larsoner:~$ pip install mne
Collecting mne
  Downloading mne-1.6.1-py3-none-any.whl.metadata (13 kB)
Successfully installed mne-1.6.1
~$ python -c "import mne; print(mne)"
<module 'mne' from '/home/larsoner/python/virtualenvs/base/lib/python3.11/site-packages/mne/__init__.py'>
~$ cd python/mne-python
~/python/mne-python$ python -c "import mne; print(mne)"
<module 'mne' from '/home/larsoner/python/mne-python/mne/__init__.py'>
~/python/mne-python$ pytest mne/io/kit/tests/test_kit.py 
=============================================================================== test session starts ================================================================================
platform linux -- Python 3.11.6, pytest-8.0.0, pluggy-1.3.0
PyQt6 6.6.0 -- Qt runtime 6.6.0 -- Qt compiled 6.6.0
MNE 1.6.1 -- /home/larsoner/python/mne-python/mne
...

You can see where mne gets imported from. Even though the version is reported incorrectly, the path is correct, and I verified that adding a raise RuntimeError to mne/__init__.py caused the RuntimeError to happen in the pytest call.

Pushed a few commits to make CIs happy then merged, thanks @JD-Zhu !