Merge pull request #30868 from mantidproject/empty_default_instrument

Empty default facility and instrument (ornlnext)

Framework/API/test/FileFinderTest.h

@@ -155,6 +155,8 @@ public:
 
   void testMakeFileNameForISIS() {
     // Set the facility
+    ConfigService::Instance().setString("default.facility", "ISIS");
+
     const FacilityInfo &facility =
         ConfigService::Instance().getFacility("ISIS");
     const InstrumentInfo &instrument = facility.instrument("HRPD");
@@ -184,10 +186,14 @@ public:
 
     TS_ASSERT_EQUALS("EFG2H00000123", FileFinder::Instance().makeFileName(
                                           "EFG2H123", instrument));
+
+    ConfigService::Instance().setString("default.facility", " ");
   }
 
   void testMakeFileNameForSNS() {
     // Set the facility
+    ConfigService::Instance().setString("default.facility", "SNS");
+
     const FacilityInfo &facility = ConfigService::Instance().getFacility("SNS");
     const InstrumentInfo &instrument = facility.instrument("CNCS");
 
@@ -212,6 +218,8 @@ public:
     // Test for REF_L (to check that the extra _ doesn't upset anything)
     TS_ASSERT_EQUALS("REF_L_666", FileFinder::Instance().makeFileName(
                                       "REF_L666", instrument));
+
+    ConfigService::Instance().setString("default.facility", " ");
   }
 
   void testGetInstrument() {

Framework/CurveFitting/test/Functions/FunctionQDependsTest.h

@@ -154,11 +154,15 @@ private:
   // return a MatrixWorkspace with detectors allowing computations of Q values
   Mantid::API::MatrixWorkspace_sptr withDetectors() {
     Mantid::DataHandling::LoadNexus loader;
+    Mantid::Kernel::ConfigService::Instance().setString("default.facility",
+                                                        "ISIS");
     loader.initialize();
     loader.setPropertyValue("Filename", "irs26173_graphite002_red");
     loader.setPropertyValue("OutputWorkspace", "irs26173");
     TS_ASSERT_THROWS_NOTHING(loader.execute());
     TS_ASSERT(loader.isExecuted());
+    Mantid::Kernel::ConfigService::Instance().setString("default.facility",
+                                                        " ");
     return Mantid::API::AnalysisDataService::Instance()
         .retrieveWS<Mantid::API::MatrixWorkspace>("irs26173");
   }

Framework/DataHandling/test/LoadILLTest.h

@@ -25,7 +25,17 @@ public:
   static LoadILLTest *createSuite() { return new LoadILLTest(); }
   static void destroySuite(LoadILLTest *suite) { delete suite; }
 
-  void tearDown() override { AnalysisDataService::Instance().clear(); }
+  void setUp() override {
+
+    ConfigService::Instance().setString("default.facility", "ILL");
+  }
+
+  void tearDown() override {
+
+    ConfigService::Instance().setString("default.facility", " ");
+
+    AnalysisDataService::Instance().clear();
+  }
 
   void checkLoader(const std::string &filename, std::string resultLoader) {
     Load alg;

Framework/DataHandling/test/LoadMcStasNexusTest.h

@@ -47,7 +47,7 @@ public:
 
     // Now set it...
     // specify name of file to load workspace from
-    inputFile = "mcstas.n5";
+    inputFile = "mcstas.h5";
     algToBeTested.setPropertyValue("Filename", inputFile);
 
     TS_ASSERT_THROWS_NOTHING(algToBeTested.execute());
@@ -95,7 +95,7 @@ public:
     outputSpace = "LoadMcStasNexusTest";
     loadMcStasNexusAlg.setPropertyValue("OutputWorkspace", outputSpace);
 
-    inputFile = "mcstas.n5";
+    inputFile = "mcstas.h5";
     loadMcStasNexusAlg.setPropertyValue("Filename", inputFile);
   }
 

Framework/DataHandling/test/LoadTest.h

@@ -34,12 +34,16 @@ public:
     m_dataSearchDirs = ConfigService::Instance().getDataSearchDirs();
 
     m_instName = ConfigService::Instance().getString("default.instrument");
+
+    ConfigService::Instance().setString("default.facility", "ISIS");
   }
 
   void tearDown() override {
 
     ConfigService::Instance().setDataSearchDirs(m_dataSearchDirs);
 
+    ConfigService::Instance().setString("default.facility", " ");
+
     ConfigService::Instance().setString("default.instrument", m_instName);
 
     AnalysisDataService::Instance().clear();

Framework/Kernel/src/ConfigService.cpp

@@ -1830,7 +1830,7 @@ const std::vector<std::string> ConfigServiceImpl::getFacilityNames() const {
 const FacilityInfo &ConfigServiceImpl::getFacility() const {
   std::string defFacility = getString("default.facility");
   if (defFacility.empty()) {
-    defFacility = "ISIS";
+    defFacility = " ";
   }
   return this->getFacility(defFacility);
 }

Framework/Kernel/src/FacilityInfo.cpp

@@ -42,10 +42,6 @@ FacilityInfo::FacilityInfo(const Poco::XML::Element *elem)
       m_zeroPadding(0), m_delimiter(), m_extensions(), m_archiveSearch(),
       m_instruments(), m_noFilePrefix(), m_multiFileLimit(100),
       m_computeResources() {
-  if (m_name.empty()) {
-    g_log.error("Facility name is not defined");
-    throw std::runtime_error("Facility name is not defined");
-  }
 
   // Fill the various fields from the XML
   fillZeroPadding(elem);

Framework/Kernel/src/InstrumentInfo.cpp

@@ -39,9 +39,6 @@ InstrumentInfo::InstrumentInfo(const FacilityInfo *f,
     : m_facility(f) {
 
   m_name = elem->getAttribute("name");
-  if (m_name.empty()) {
-    throw std::runtime_error("Instrument name is not defined");
-  }
 
   // The string to separate the instrument name and the run number.
   m_delimiter = elem->getAttribute("delimiter");

Framework/Properties/Mantid.properties.template

@@ -10,7 +10,7 @@
 #          we will never alter your user.properties file so settings there are safe.
 
 # Set a default facility
-default.facility = ISIS
+default.facility =
 
 # Set a default instrument
 default.instrument =

Framework/PythonInterface/test/python/mantid/plots/datafunctionsTest.py

@@ -17,7 +17,7 @@ import numpy as np
 import mantid.api
 import mantid.plots.datafunctions as funcs
 from unittest.mock import Mock
-from mantid.kernel import config
+from mantid.kernel import config, ConfigService
 from mantid.plots.utility import MantidAxType
 from mantid.simpleapi import (AddSampleLog, AddTimeSeriesLog, ConjoinWorkspaces,
                               CreateMDHistoWorkspace, CreateSampleWorkspace,
@@ -877,9 +877,11 @@ class DataFunctionsTest(unittest.TestCase):
         self.assertTrue(isinstance(bin_indices, range))
 
     def test_get_bin_indices_returns_a_numpy_ndarray_with_monitors(self):
+        ConfigService.Instance().setString("default.facility", "ISIS")
         ws = LoadRaw("GEM40979", SpectrumMin=1, SpectrumMax=102)
         bin_indices = funcs.get_bin_indices(ws)
         self.assertTrue(isinstance(bin_indices, np.ndarray))
+        ConfigService.Instance().setString("default.facility", " ")
 
 
 if __name__ == '__main__':

Framework/PythonInterface/test/python/plugins/algorithms/WorkflowAlgorithms/ReflectometryILLPreprocessTest.py

@@ -5,6 +5,7 @@
 #   Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS
 # SPDX - License - Identifier: GPL - 3.0 +
 
+from mantid.kernel import config
 from mantid.api import mtd
 from mantid.simpleapi import ReflectometryILLPreprocess
 import numpy.testing
@@ -15,6 +16,20 @@ import math
 
 
 class ReflectometryILLPreprocessTest(unittest.TestCase):
+    def setUp(self):
+        self._facility = config['default.facility']
+        self._instrument = config['default.instrument']
+
+        config['default.facility'] = 'ILL'
+        config['default.instrument'] = 'D17'
+
+    def tearDown(self):
+        if self._facility:
+            config['default.facility'] = self._facility
+        if self._instrument:
+            config['default.instrument'] = self._instrument
+        mtd.clear()
+
     def tearDown(self):
         mtd.clear()
 
@@ -132,5 +147,6 @@ class ReflectometryILLPreprocessTest(unittest.TestCase):
         self.assertEqual(outWS.getAxis(0).getUnit().caption(), 'Wavelength')
         self.assertEqual(mtd.getObjectNames(), [])
 
+
 if __name__ == "__main__":
     unittest.main()

Framework/Reflectometry/test/ReflectometryReductionOneAuto2Test.h

@@ -75,6 +75,16 @@ private:
   }
 
 public:
+  void setUp() override {
+    Mantid::Kernel::ConfigService::Instance().setString("default.facility",
+                                                        "ISIS");
+  }
+
+  void tearDown() override {
+    Mantid::Kernel::ConfigService::Instance().setString("default.facility",
+                                                        " ");
+  }
+
   // This pair of boilerplate methods prevent the suite being created statically
   // This means the constructor isn't called when running other tests
   static ReflectometryReductionOneAuto2Test *createSuite() {

Framework/Reflectometry/test/ReflectometryReductionOneAuto3Test.h

@@ -64,6 +64,16 @@ private:
   };
 
 public:
+  void setUp() override {
+    Mantid::Kernel::ConfigService::Instance().setString("default.facility",
+                                                        "ISIS");
+  }
+
+  void tearDown() override {
+    Mantid::Kernel::ConfigService::Instance().setString("default.facility",
+                                                        " ");
+  }
+
   // This pair of boilerplate methods prevent the suite being created statically
   // This means the constructor isn't called when running other tests
   static ReflectometryReductionOneAuto3Test *createSuite() {

Testing/SystemTests/lib/systemtests/systemtesting.py

@@ -34,7 +34,7 @@ import importlib.util
 import inspect
 from mantid.api import FileFinder
 from mantid.api import FrameworkManager
-from mantid.kernel import config, MemoryStats
+from mantid.kernel import config, MemoryStats, ConfigService
 from mantid.simpleapi import AlgorithmManager, Load, SaveNexus
 import numpy
 import platform
@@ -1199,6 +1199,10 @@ class MantidFrameworkConfig:
         # Make sure we only save these keys here
         config.reset()
 
+        # With the default facility changed from ISIS to nothing (EMPTY),
+        # the following setting is put in place to avoid failure of tests
+        ConfigService.Instance().setString("default.facility", "ISIS")
+
         # Up the log level so that failures can give useful information
         config['logging.loggers.root.level'] = self.__loglevel
         # Set the correct search path

docs/source/conf.py

@@ -101,6 +101,11 @@ pygments_style = 'sphinx'
 # settings after each test.
 mantid_init_config_keys = ('datasearch.directories', 'defaultsave.directory', 'default.facility',
                            'default.instrument')
+
+# With the default facility changed from ISIS to nothing (EMPTY),
+# the following setting is put in place to avoid failure of tests
+ConfigService.Instance().setString("default.facility", "ISIS")
+
 mantid_config_reset = [
     "_cfg['{0}'] = '{1}'".format(k,
                                  ConfigService.Instance()[k]) for k in mantid_init_config_keys

docs/source/release/v6.1.0/mantidworkbench.rst

@@ -11,6 +11,8 @@ New and Improved
 - New plot interactions: Double click a legend to hide it, double click a curve to open it in the plot config dialog.
 - It is now possible to overplot bin data from the matrix workspace view.
 - Improved the performance of the table workspace display for large datasets
+- A new empty facility with empty instrument is the default facility now, and
+  user has to select their choice of facility (including ISIS) and instrument for the first time
 
 Bugfixes
 --------
@@ -19,5 +21,8 @@ Bugfixes
 - For the elliptical shell of integrated peaks, the background is correct when plotting with varying background thicknesses
 - Fixed a bug which occurred when switching to a log scale in sliceviewer with negative data.
 - Fixed a bug that use wrong help links in certain interfaces
+- If the facility in Mantid.user.properties is empty, it is consistently reflected as empty in the GUI
+- First time dialog box will not appear recurrently, if user selected their choice of facility
+  and instrument at least once and checked "Do not show again until next version"
 
 :ref:`Release 6.1.0 <v6.1.0>`

instrument/Facilities.xml

 <?xml version="1.0" encoding="UTF-8"?>
 <facilities>
 
+<!--  EMPTY  -->
+<facility name=" " FileExtensions=" ">
+  <timezone></timezone>
+
+  <instrument name=" ">
+    <technique> </technique>
+  </instrument>
+
+</facility>
+
 <facility name="ISIS" zeropadding="5" FileExtensions=".nxs,.raw,.sav,.n*,.s*,.add,.nxspe">
 
   <archive>

qt/applications/workbench/workbench/widgets/settings/general/test/test_general_settings.py

@@ -105,7 +105,7 @@ class GeneralSettingsTest(unittest.TestCase):
 
         mock_ConfigService.setFacility.assert_called_once_with(new_facility)
 
-        self.assertEqual(43, presenter.view.instrument.count())
+        self.assertEqual(1, presenter.view.instrument.count())
 
     def test_setup_confirmations(self):
         presenter = GeneralSettings(None)

qt/scientific_interfaces/test/ISISReflectometry/Runs/RunsPresenterTest.h

@@ -40,6 +40,16 @@ using testing::ReturnRef;
 //=====================================================================================
 class RunsPresenterTest : public CxxTest::TestSuite {
 public:
+  void setUp() override {
+    Mantid::Kernel::ConfigService::Instance().setString("default.facility",
+                                                        "ISIS");
+  }
+
+  void tearDown() override {
+    Mantid::Kernel::ConfigService::Instance().setString("default.facility",
+                                                        " ");
+  }
+
   // This pair of boilerplate methods prevent the suite being created statically
   // This means the constructor isn't called when running other tests
   static RunsPresenterTest *createSuite() { return new RunsPresenterTest(); }

scripts/test/Muon/utilities/load_utils_test.py

@@ -8,7 +8,8 @@ import Muon.GUI.Common.utilities.load_utils as utils
 import os
 import unittest
 
-from mantid import simpleapi, ConfigService
+from mantid import simpleapi
+from mantid.kernel import ConfigService
 from mantid.api import AnalysisDataService, ITableWorkspace
 
 
@@ -57,6 +58,7 @@ class MuonFileUtilsTest(unittest.TestCase):
         self.assertEqual(instrument, 'MUSR')
 
     def test_that_load_dead_time_from_filename_places_table_in_ADS(self):
+        ConfigService.Instance().setString("default.facility", "ISIS")
         filename = 'MUSR00022725.nsx'
 
         name = utils.load_dead_time_from_filename(filename)
@@ -64,8 +66,10 @@ class MuonFileUtilsTest(unittest.TestCase):
 
         self.assertEqual(name, 'MUSR00022725.nsx_deadtime_table')
         self.assertTrue(isinstance(dead_time_table, ITableWorkspace))
+        ConfigService.Instance().setString("default.facility", " ")
 
     def test_load_workspace_from_filename_for_existing_file(self):
+        ConfigService.Instance().setString("default.facility", "ISIS")
         filename = 'MUSR00022725.nsx'
         load_result, run, filename, _ = utils.load_workspace_from_filename(filename)
 
@@ -74,6 +78,7 @@ class MuonFileUtilsTest(unittest.TestCase):
         self.assertEqual(load_result['MainFieldDirection'], 'Transverse')
         self.assertAlmostEqual(load_result['TimeZero'], 0.55000, 5)
         self.assertEqual(run, 22725)
+        ConfigService.Instance().setString("default.facility", " ")
 
     def test_load_workspace_from_filename_for_file_path(self):
         filename = 'PSI'+ os.sep + 'run_1529_templs0.mon'
@@ -85,5 +90,5 @@ class MuonFileUtilsTest(unittest.TestCase):
         self.assertTrue(filename in alg.getProperty("Filename").value)
 
 
-if __name__ == '__main__':
+if __name__ == "__main__":
     unittest.main(buffer=False, verbosity=2)