ENH: new coreg gui features (part 2)

[GuillaumeFavelier]

This PR is based on #10015 and adds the following:

• improve status bar, add infos about omitted points (i.e. how many are left), infos about save/load trans files
• add units (mm.) to grow hair & omit distance widgets
• display (mean/min/max) dig/mri distance estimation during fitting
• display axes
• display distance to fiducials in mm.

It's part of #8833

Can only be merged after #10015

[larsoner]

Looking forward to this now that #10015 is in!

[GuillaumeFavelier]

for some reason the message "Fit fiducials" is cropped for me in the status

Assuming that your issue is related to _status_bar_show_message and the stretch parameter, I reworked the function to use QTimer instead. Can you try it again @agramfort ?

[larsoner]

Looks nice!

One comment is that when ICP runs, the distances are now shown in two places: in the fitting pane, and another in the status bar:

In general, people should ideally have one place they can always look for any given type of information. For distances in particular, I'd keep it all in the right "Fitting" bar. Then the status bar can just show stuff about the number of iterations, and then at the end how many iterations were done and how long it took. Same thing would then goes for "Fitting ICP - Iteration N". This shows up in the 3D plot, but would now be better just in the status bar. In other words:

1. Status bar = what is being done, or what was last done (and how long it took)
2. Right "Fitting" panel = all the distance information that is deemed useful

I reworked the function to use QTimer instead.

I'd prefer actually that whatever the last message was just stays put. I don't see much advantage to removing the last status message displayed. It's even helpful to have it stay forever in some cases to remind people what the last thing they did was!

[larsoner]

... also when running at some point I got:

Traceback (most recent call last):
  File "/home/larsoner/python/mne-python/mne/gui/_coreg.py", line 865, in _fit_icp
    self._last_log +
TypeError: unsupported operand type(s) for +: 'NoneType' and 'str'

[agramfort]

idem ❯ mne coreg -s sample -d ~/mne_data/MNE-sample-data/subjects --interaction=trackball -f ~/mne_data/MNE-sample-data/MEG/sample/sample_audvis_raw.fif Traceback (most recent call last): File "/Users/alex/work/src/mne-python/mne/gui/_coreg.py", line 865, in _fit_icp self._last_log + TypeError: unsupported operand type(s) for +: 'NoneType' and 'str' Fatal Python error: Aborted Current thread 0x000000010b34cdc0 (most recent call first): File "/Users/alex/work/src/mne-python/mne/viz/backends/_utils.py", line 163 in _qt_app_exec File "/Users/alex/work/src/mne-python/mne/gui/_coreg.py", line 252 in __init__ File "<decorator-gen-549>", line 24 in __init__ File "/Users/alex/work/src/mne-python/mne/gui/__init__.py", line 192 in coregistration File "<decorator-gen-537>", line 24 in coregistration File "/Users/alex/work/src/mne-python/mne/commands/mne_coreg.py", line 89 in run File "/Users/alex/work/src/mne-python/mne/commands/utils.py", line 107 in main File "/Users/alex/miniconda3/bin/mne", line 33 in <module> [1] 15062 abort mne coreg -s sample -d ~/mne_data/MNE-sample-data/subjects -f

…

[GuillaumeFavelier]

In the end, I found that the simplest solution is to display all the messages in the status bar with _display_message().
I also moved all the 'distance' code to _update_distance_estimation().
These changes should make for a cleaner interface.

[GuillaumeFavelier]

And this method is actually compatible with the notebook backend (status bar post-processed in blue):

It feels a bit cramped but it is a work in progress after all

[GuillaumeFavelier]

what about now @larsoner, @agramfort ?

[GuillaumeFavelier]

I changed it back to WIP because @agramfort found 2 bugs:

1. the distances are not updated during fitting
2. the distances should take into account only the points that are not excluded by 'Omit' (instead of all at the moment)

[GuillaumeFavelier]

the distances are not updated during fitting

I do not have this behaviour on my linux and I checked that self._renderer._process_events() is called at each iteration of fit_icp(). This could be yet another inconsistency of Qt on MacOS?

I will start by looking into 2).

[GuillaumeFavelier]

I added a few calls to _update_distance_estimation(), this should be enough for 2).

I tried 2d4b4b7 for MacOS based on what is used on _pyvista.py 🤞

[GuillaumeFavelier]

371b86a is my latest attempt at fixing 1) on MacOS

[GuillaumeFavelier]

Okay, it's better than I thought. So @agramfort, in order to move forward with this one, I can report that there could be a delay with updating the distances on MacOS on #8833

I was able to check that the status bar and the render view are updated correctly so it's ready to go on my side.

[larsoner]

Just a couple of small tweaks / ideas we can keep in mind moving forward. Works great so I'll merge, thanks @GuillaumeFavelier !

[larsoner]

No longer need OrderedDict because we are 3.7+. I think our existing ones are mostly there for cruft reasons

[larsoner]

In the future we should properly disable in a UI way (button.setEnabled(False) in Qt parlance for example) the things that people are not supposed to use