Fix Inelastic Symmetrise Preview crash #38051
Conversation
Removes this member variable because it is never set to false anywhere else in the code. It means this class can be simplified slightly.
robertapplin
added
Bug
robertapplin
force-pushed
the
38042-symmetrise-preview-crash
branch
from
324cda6 to
74a8a67
Compare
There was a problem hiding this comment.
Thanks for fixing this issue.
I have tested following instructions, Mantid no longer crashes when clicking Preview button after clearing the ADS.
Otherwise, you said autoload was sending confusing messages. There other inelastic interfaces with autoload enabled (now by default) like Iqt. When you try to run the algorithm without data on the ADS, it prompts an error message, but then it loads the data.
Could we open an issue to handle better this case in inelastic interfaces having autoload enabled?
| // files were found | ||
| emit filesFound(); | ||
| } | ||
| emit filesAutoLoaded(); |
There was a problem hiding this comment.
Sorry if I'm not following the code and PR comments very well, but I just wanted to double check that this change is what's intended - it looks like the code here previously used to check whether auto load was enabled/requested, but now it looks like it will always perform the auto-load regardless?
There was a problem hiding this comment.
Thanks for checking. I found out that the m_autoLoad option is always set to true, and so I decided to do a small bit of simplification for this class by removing the option entirely. The only place where we might not want to do auto-loading is now in the isValid function, and I have provided an option which can be passed to this function directly
There was a problem hiding this comment.
Ah, great, thanks for explaining, that all sounds good then!
Description of work
This PR fixes a crash on the Symmetrise tab of the Inelastic Data Processor interface after clearing the ADS and clicking the Preview button. The tab will now not proceed with processing if it can't find the loaded workspace in the ADS. The auto loading feature has also been disabled when validating the DataSelector on the Symmetrise tab because it was causing multiple confusing messages.
Fixes #38042
To test:
Follow the instructions on the attached issue
This does not require release notes because it is a regression since the last release
Reviewer
Please comment on the points listed below (full description).
Your comments will be used as part of the gatekeeper process, so please comment clearly on what you have checked during your review. If changes are made to the PR during the review process then your final comment will be the most important for gatekeepers. In this comment you should make it clear why any earlier review is still valid, or confirm that all requested changes have been addressed.
Code Review
Functional Tests
Does everything look good? Mark the review as Approve. A member of
@mantidproject/gatekeeperswill take care of it.Gatekeeper
If you need to request changes to a PR then please add a comment and set the review status to "Request changes". This will stop the PR from showing up in the list for other gatekeepers.