ENH: Display a dialog to save trans and fids before quitting the coreg app

[GuillaumeFavelier]

This PR basically rewrites the closeEvent() function to allow displaying a dialog in the coreg app in case the user did not save trans.

It is still work in progress. In its current state, it works but the code to achieve it needs refactoring. Plus it does not save the fids yet.

Closes #10236

It's an item of #8833

[GuillaumeFavelier]

output.mp4

[GuillaumeFavelier]

I will extract the changes made to the GUI API in another PR. I also have to work on a notebook alternative and testing.

[larsoner]

If you haven't started, I think it's okay to do the notebook separately. The Qt interface is much more commonly used here

[GuillaumeFavelier]

Okay then, let's go with Qt first.

[GuillaumeFavelier]

@larsoner I added a Discard button and fixed the trans_modified and fids_modified logic again. I tried locally and it should work as intended. Sorry for spamming but could you try it again when you have the time?

[larsoner]

During one interaction I got a segfault caused by:

  File "/Users/larsoner/python/mne-python/mne/viz/backends/_qt.py", line 64, in func
    callback(button_name)
  File "/Users/larsoner/python/mne-python/mne/gui/_coreg.py", line 1740, in _close_dialog_callback
    assert button_name == "Discard"
AssertionError

It was for MRI fiducials, and the button was labeled "Don't save". I think "Discard" is more common in UI designs.

Also, any Qt event handler should be wrapped with @safe_event so that, even if there are bugs like this, the UI does not segfault.

[larsoner]

When I load, lock fids, set to uniform scale mode, then set scaling to 99 instead of 100, I get a box saying that the head<->MRI transform is not saved. This is actually not correct -- it is the head<->MRI trans I loaded, what I didn't save was the "scaled subject MRI". So I think you need to track three states:

1. MRI fids modified
2. trans modified
3. MRI scale modified

Another point -- when the dialog comes up, if I click "Save", it correctly pulls up a save dialog. But if I then hit "cancel" on the system dialog -- and thus don't save anything -- the UI still exists. I think it should only exit if the user actually saved a file. In this sense, the cancel of the system dialog works like the cancel of the previous dialog, and the only way a user can "lose their work" is if they actually click the "Discard" button.

[GuillaumeFavelier]

the button was labeled "Don't save".

Oh no, on my system the name is Discard so this button.text() is not standard or the button itself is not.

So I think you need to track three states

I see

the only way a user can "lose their work" is if they actually click the "Discard" button.

Makes sense

[larsoner]

Oh no, on my system the name is Discard so this button.text() is not standard or the button itself is not.

Okay so it's a standard (my version of) macOS thing, that's fine -- just update the assert or remove it

[GuillaumeFavelier]

Hopefully it's good enough now 🙏

[larsoner]

LGTM +1 for merge when it comes back green

[larsoner]

I still got:

(base) bunker:MNE-sample-data larsoner$ mne coreg -s sample -d subjects --fif MEG/sample/sample_audvis_raw.fif --trans MEG/sample/sample_audvis_raw-trans.fif 
Traceback (most recent call last):
  File "/Users/larsoner/python/mne-python/mne/viz/backends/_qt.py", line 67, in func
    callback(button_name)
  File "/Users/larsoner/python/mne-python/mne/gui/_coreg.py", line 1765, in _close_dialog_callback
    assert button_name == "Discard"
AssertionError
Fatal Python error: Aborted

when I tweaked a fiducial, hit the X to close, then hit "Don't save". I pushed a commit to get rid of the assertion

[GuillaumeFavelier]

What is the exact value of button_name that is compared to 'Discard' on your system?

[larsoner]

I put it in the code commented out

[larsoner]

... more specifically:

        #     assert button_name in ("Discard", "Don't Save"), button_name

[GuillaumeFavelier]

You checked windows to make sure this is correct there?

While testing https://github.com/hoechenberger/mne-installers on Windows, I ran mne coreg -s sample -d subjects:

So it uses "Discard" too!

---

I also ran into other bugs specific to Windows but now I can debug properly and fix them in another PR.

• crash during _configure_status_bar
• hangs while using _CheckInside

[larsoner]

I also ran into other bugs specific to Windows but now I can debug properly and fix them in another PR.

Great -- I would say these are high priority since they will probably make the coreg gui unusable on Windows :(

It now works for me on macOS, thanks @GuillaumeFavelier !

[GuillaumeFavelier]

I'll work on windows for a while to fix those 👍