Adding group inputs to SaveNXcanSAS algorithm

[adriazalvarez]

Description of work

This PR refactors the SaveNXcanSAS algorithm to accept group workspaces as inputs. This prepares the ground for a future algorithm for saving polarized SANS data that will need the feature. More information on : #38504

Summary of work

To save groups there are three main changes to notice:

• The InputWorkspace property accepts a pointer to the base class Workspace that can be downcasted to a matrix workspace or group workspace. To be able to still filter appropiate compatible data and distinguish between group and matrix workspace I have added a custom LambdaValidator that does unit and compatible bins validation to each workspace in the selected input groups, that will filter appropiate groups. Additionally, it also filters compatible workspaces.
• To handle groups in the algorithms, I have overridden the processGroups methods from the Algorithm base class, so that it will call exec when it needs to process matrix workspaces and processGroups for processing group workspaces.
• The default behaviour for groups (although this can be discussed further) is to save each workspace of the group in an individual file and add a digit at the end of the selected filename to distinguish between files. Although we could add another input property for the algorithm with a custom suffix for group inputs.

Fixes #38504.

Further detail of work

To test:

• Code review.
• All tests are passing.
• Check doc changes are appropriate.
• Run the script and check that data is saved as intended (This uses CLI to reduce data and the training course data set):

# import mantid algorithms, numpy and matplotlib
from mantid.simpleapi import *
import matplotlib.pyplot as plt
import numpy as np
from sans.command_interface.ISISCommandInterface import *

LOQ()
MaskFile('MaskFile.toml')
AssignSample('LOQ74044.nxs')
TransmissionSample('LOQ74024.nxs', 'LOQ74014.nxs')
AssignCan('LOQ74019.nxs')
TransmissionCan('LOQ74020.nxs', 'LOQ74014.nxs')
#Performs reduction
ws_out=WavRangeReduction(2.2, 10)

ws_group=GroupWorkspaces(ws_out)

# saved in default mantid save directory

# this ought to save two separate files with suffices 00 and 01 and name test_group
SaveNXcanSAS(ws_group, 'test_group')

#this saves individual file with name test_file
SaveNXcanSAS(ws_out[0], 'test_file')

---

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

• Is the code of an acceptable quality?
• Does the code conform to the coding standards?
• Are the unit tests small and test the class in isolation?
• If there is GUI work does it follow the GUI standards?
• If there are changes in the release notes then do they describe the changes appropriately?
• Do the release notes conform to the release notes guide?

Functional Tests

• Do changes function as described? Add comments below that describe the tests performed?
• Do the changes handle unexpected situations, e.g. bad input?
• Has the relevant (user and developer) documentation been added/updated?

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.

[sf1919]

I'm removing the high priority as the cppcheck hackathon starts today. This PR may well attract conflicts so it might be best to wait until Thursday when the hackathon has finished before dealing with them.

[github-actions]

👋 Hi, @adriazalvarez,

Conflicts have been detected against the base branch. Please rebase your branch against the base branch.

[adriazalvarez]

Lots of conflicting PRs will be merged today and tomorrow, so I will wait until then to fix the conflicts.

[cailafinn]

Just a couple of code notes.

Working well functionally, just noticed that the files produced by the script don't have the .nxs extension. Is this the same as the original behaviour or is this an introduced change?

[adriazalvarez]

@cailafinn Yes, originally the algorithm was not producing any extension when the filename property does not specify any. I have quickly added a helper function to add extension by default (.h5), but if it's not something we want here I can revert it.

[cailafinn]

Just one more note on the filepath handling.

[cailafinn]

It looks like you could use the NX_CANSAS_EXTENSION you defined on L33 here.

It may also be worth editing this method and the parameter in saveSingleWorkspaceFile to handle baseFilename as a std::filesystem::path. This would allow us to use things like has_extension to only append the .h5 extension (with replace_extension) if the user hasn't supplied one already.

[cailafinn]

This looks good to me now. Functionally tested using the provided script, producing files as expected. File names are correctly indexed. Code looking good.

@MialLewis This is now ready for gatekeeping when you are.

[MialLewis]

Code looks really good, thanks.

Two queries to respond to.

[MialLewis]

If a test was to error and the clean up call at the end missed, will subsequent tests fail now this has been removed?

If this could happen it could make it harder to identify and resolve any bugs revealed by the tests.

[adriazalvarez]

As far as I could see, when constructing the NXcanSASTestParameters struct, a new temporary file was created on the respective system temporary folder, the file was immediately removed because all that was needed for saving the NXcansas file was the path of the file. I have instead used the method Poco::TemporaryFile::tempName() on the NXcanSASTestHelper.h file to just retrieve the path of a temporary file in the folder without creating one, so there is no need to delete it just after creation.
If a test fails, there won't be a cascading effect, as the temporary names are uniquely generated and collisions would be extremely rare. If for some reason a temporary file is generated but the test fails, the file would be on a system temporary folder, and probably removed automatically by the system shortly after.

[sf1919]

We are trying to move away from Poco with File and Path. I know @jclarkeSTFC has been working on that for #37868 . I'm wondering if we should also be avoiding using Poco::TemporaryFile too.

[MialLewis]

https://en.cppreference.com/w/cpp/io/c/tmpnam seems like a direct replacement?

I'm aware the name collision is unlikely, but as we know it is possible I'm not sure we should be introducing it.