mantid
mantid copied to clipboard
Modify ISIS SANS code to no longer use deprecated import
There are various places in the code that import the wrong package and generate the warning
This ISIS Command Interface is deprecated.
Please change 'import ISISCommandInterface' or 'from ISISCommandInterface'to use 'sans.command_interface.ISISCommandInterface' instead.
This is an unnecessary warning that is generated by the following files
$ rg "import ISISCommandInterface" -t py -l
scripts/test/SANS/SansIsisGuiSettings.py
scripts/test/SANS/SANSReducerTest.py
scripts/test/SANS/SANSCentreFinderTest.py
scripts/test/SANS/SANSCommandInterfaceTest.py
scripts/test/SANS/SANSReductionStepsUserFileTest.py
scripts/SANS/ISISCommandInterface.py
Testing/SystemTests/tests/framework/ISIS/SANS/SANS2D/SANS2DSearchCentreGUI.py
Testing/SystemTests/tests/framework/ISIS/SANS/WORKFLOWS/SANSMergedDetectorsTest.py
Testing/SystemTests/tests/framework/ISIS/SANS/SANS2D/SANS2DReductionGUIAdded.py
Testing/SystemTests/tests/framework/ISIS/SANS/SANS2D/SANS2DSlicing.py
Testing/SystemTests/tests/framework/ISIS/SANS/SANS2D/SANS2DReductionGUI.py
Testing/SystemTests/tests/framework/ISIS/SANS/WORKFLOWS/SANSLoadersTest.py
Testing/SystemTests/tests/framework/ISIS/SANS/LOQ/SANSLOQAddBatch.py
Testing/SystemTests/tests/framework/ISIS/SANS/LOQ/SANSLOQReductionGUI.py
Worse still is that several of these imports rename the package to a poorly chosen name
import ISISCommandInterface as i
where a clearer option has already been suggested in some of the files
import ISISCommandInterface as command_iface
As part of this we should update the documentation for the scripting interface, as that is still showing examples that include the deprecated import: https://docs.mantidproject.org/nightly/techniques/ScriptingSANSReductions.html.
Also, when the import is used in Mantid Workbench I'm not seeing the warning logged in the messages pane - I'm not sure if this is supposed to be the case or not. We had a scientist that tried to use the deprecated import when loading a TOML user file, which is only supported by the newer import, but they were unable to resolve this themselves because the warning about the deprecated import doesn't seem to display.
After an initial investigation by @yusufjimoh, we think this one is a bit more complex than it initially appears. For example, if you change the import in scripts/test/SANS/SansIsisGuiSettings.py
then the new version of the interface has a different implementation that causes the tests to break. Although the deprecated version of the interface throws a warning on import, I can't actually see anything about the switch to the new interface in our documentation and all examples in the docs currently use the old import.
I think this issue will involve investigating the background of the deprecation more fully, including with our scientists, and checking if we still need to support the deprecated version or if it could now be removed (if not then test coverage would still be required). It would also involve making sure that documentation is updated appropriately and that there is appropriate test coverage of the new version of the interface.