-
Notifications
You must be signed in to change notification settings - Fork 126
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
Add additional support for saving data using the ISIS Sans Command Interface #38618
base: main
Are you sure you want to change the base?
Conversation
…d BatchReduce and WavReduction functions as well as reformat presentation of various visual elements
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this, tested and working well.
Two very minor comments.
save_algs = [] | ||
if save_algorithms: | ||
for key in save_algorithms.keys(): | ||
if key == "SaveRKH": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you consider updating this with structural pattern matching here?
I agree with your decision to roll back from a list to the pre-existing dictionary, it's probably not worth breaking existing scripts for the minor code quality upgrade!
""" | ||
Mainly internally used by BatchMode. Provides the save settings. | ||
Mainly used internally by BatchReduce and WavRangeReduction. Prepares the save state on the director |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've removed it from BatchReduce
(a good improvement), though I appreciate WavRangeReduction
is called in its place?
Description of work
This PR modifies the
WavRangeReduction
function to allow it to have the same input for saving algorithms (A dictionary of save algorithms) as theBatchReduce
function. This will give more flexibility to scientist using the CLI to save without having to create a batch file, asBatchReduce
basically prepares the data to be sent toWavRangeReduction
.To do this change, I have refactored the
set_save
function, which technically can be used as a command too, to process the dictionary of save algorithms.Fixes #38502.
Further detail of work
There are two things to notice that can be further discussed in the review:
BatchReduce
) and this would their scripts.WavRangeReduction
and changing the example test to use the new interface with data from the LOQ demo in the Training Course Set data. This also uses an example Mask file with.toml
extension , which I think is the currently supported format in the SANS group in ISIS.deprecated
warning if trying to use the old interface.To test:
../ScriptingSANSReductions.html
, everything should be well formatted and typo free.Added release notes
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/gatekeepers
will 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.