Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added ADSObserver to MSlice #1041

Merged
merged 18 commits into from
Dec 13, 2024
Merged

Added ADSObserver to MSlice #1041

merged 18 commits into from
Dec 13, 2024

Conversation

SilkeSchomann
Copy link
Collaborator

Description of work:
So far MSlice interacted with the ADS in a one-sided way. Some of its workspaces were stored in the ADS and occasionally deleted, but changes to the ADS from the Mantid side ignored. This sometimes led to missing workspaces in MSlice when the respective workspace was deleted on the Mantid side. However, the respective workspace would still get displayed in MSlice and cause a crash when selected.

This PR introduces an ADS observer that modifies MSlice workspaces on delete, clear and rename, respectively.
In addition, two small bugs were fixed that became obvious when testing these changes:

  1. When taking an interactive cut and closing the interactive cut window without saving the cut to the workspace, a cut was showing up in the MD Histo tab.
  2. When clearing all workspace, there were still workspaces shown in the MD Histo tab that were not available anymore and caused crashes on selection. This was particularly visible after a clear() from the ADS that should cause all workspaces in all tabs to get cleared.

To test:

With workspaces saved to the ADS from the MSlice interface, check that the ADS commands clear, delete and rename work as expected in MSlice when run from Mantid.

Fixes #839.

Copy link
Contributor

@GuiMacielPereira GuiMacielPereira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code is well written and easy to understand. Unit tests were added.
I tested this change manually by renaming, clearing and deleting workspaces in the ADS and confirmed that they are correctly updated in mslice. I tested that when closing an interactive cut without saving the workspace is not accidentally saved to the Histo tab; Checked that the clear button does clear all workpaces in ADS and mslice (even the hidden ones).

I also did some cuts and workspace operations, saved them to the ADS from mslice, renamed and deleted them and checked that they were successfully being updated back into mslice.

I also noticed that renaming or deleting hidden mslice (__MSL) workspaces in the ADS does not update the workspace on mslice, and might cause some issues. However, these workspaces are hidden because the user is not supposed to interact with them in the ADS anyway (and it's something that can be dealt with in another PR)

@SilkeSchomann SilkeSchomann merged commit b54db7f into main Dec 13, 2024
2 checks passed
@SilkeSchomann SilkeSchomann deleted the 839_add_ads_observer branch December 13, 2024 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

User issues ADS.clear() in script and breaks MSlice
2 participants