neuroinformatics-unit / datashuttle

Tool for the creation, validation and transfer of neuroscience project folders.
https://datashuttle.neuroinformatics.dev/
BSD 3-Clause "New" or "Revised" License
14 stars 3 forks source link

Fix error from new textualize from `dismiss()` in `call_after_refresh`. #412

Closed JoeZiminski closed 1 week ago

JoeZiminski commented 1 week ago

This PR is a bug fix, which addresses a new error that seems to have been caused by a textualize update.

Because of the way the event loop works, as far as I can tell whenever a new Screen is made, it takes control of the application. This means if we are transferring data, if we want to show a 'Transferring' screen, then we need to pass a lot of options to a dialog class which itself runs the transfer. This approach seems a little strange, because the 'Transferring' window is just a lightweigh screen, whereas all options for the transfer are set and coordinated in the Transfer tab, so it makes sense for the transfer logic to be in the transfer-tab class.

To allow this, FinishTransferScreen provides an OK (proceed with transfer) or Cancel (dont transfer) button. If OK is pressed, previously is would change the message to 'Transferring' and call self.dismiss(True) within a call_after_refresh, that would (after GUI update) dismiss and trigger the callback function transfer_data. In recent versions, this reaises an error.

As a workaround, a new (slightly messier) workflow means FinishTransferScreen directly calls the transfer function, and the transfer function tears down FinishTransferScreen itself. This is documented in a docstring in this PR.

An alterantive approach would be: 1) in the transfer tab, call FinishTransferScreen and wait for response with push_screen_wait 2) if True, call a run_transfer_data function. This will create a RunTransferDialog class that shows 'Transferring', but will also require all options from the transfer tab to be passed to it, including the interface. Then the code to run the transfer will be run from this dialog window. 3) This push screen has a callback which handles displaying if the transfer worked or not.

Overall I like this approach less because a lot of the core transfer code is moved into an obscure dialog, rather on the transfer tab, which has the function to coordinate and run the transfer.

See #431 for a much nicer solution to implement ASAP.