Ticket #8755 (closed: fixed)
Remove dead code
Reported by: | Owen Arnold | Owned by: | Owen Arnold |
---|---|---|---|
Priority: | minor | Milestone: | Release 3.1 |
Component: | Reflectometry | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | #8765 | Tester: | Jay Rainey |
Description
This issue has been raised by some confused intrument scientists. Somehow, we have duplicate versions of the reflectometry gui code. New modifications are being made to the scripts in scripts/ui/interface/reflectometer, but the there is a duplicate refl_gui.py file still sitting around in Reflectometry/isis_relgui. Even worse, the new gui code-behind file seems to use modules in the old location!
- Delete the old reflgui.py file,
- Move the used modules next to the new reflgui.py file
- Delete the isis_reflgui directory completely
Change History
comment:2 Changed 7 years ago by Owen Arnold
refs #8755. Restructure modules and delete others.
Changeset: 0ff33822a15513a93bf450971708dd3607d57fb4
comment:3 Changed 7 years ago by Owen Arnold
refs #8755. Better not to have the settings.py hidden in ui dirs.
Move to the isis_reflectometry package instead. Fix tests.
Changeset: 0924fb4459321736d0ddaf1f2e23bb3639510e39
comment:4 Changed 7 years ago by Owen Arnold
Updated documentation here http://www.mantidproject.org/ISIS_Reflectometry_GUI#Settings
comment:5 Changed 7 years ago by Owen Arnold
Tester. You should be able to use Keith's test instructions http://www.mantidproject.org/Using_ISIS_Reflectometry_GUI#Testing for testing that nothing has broken as part of the move. This was a refactoring step. The only thing that should HAVE to move is the location of the settings file (documented here http://www.mantidproject.org/ISIS_Reflectometry_GUI#Settings)
comment:6 Changed 7 years ago by Owen Arnold
- Status changed from inprogress to verify
- Resolution set to fixed
comment:7 Changed 7 years ago by Owen Arnold
The settings.py and settings.xml information could be completely removed (and should) be, once #8691 is complete, as the settings.xml file duplicated information in the facilities.xml file.
comment:8 Changed 7 years ago by Jay Rainey
- Status changed from verify to verifying
- Tester set to Jay Rainey
comment:9 Changed 7 years ago by Jay Rainey
- Status changed from verifying to reopened
- Resolution fixed deleted
I tried to open the interface and the following error was produced:
ImportError: No module named isis_reflgui.settings at line 6 in '<Interface>' caused by line 5 in '/opt/Mantid/scripts/Interface/ui/reflectometer/refl_gui.py' caused by line 8 in '/opt/Mantid/scripts/Interface/ui/reflectometer/refl_save.py' caused by line 8 in '/opt/Mantid/scripts/Reflectometry/isis_reflectometry/saveModule.py'
I looked into it, and it appears you forgot to update the import in refl_save and saveModule when you re-named the file. E.g., from isis_reflgui to isis_reflectometry.
comment:10 Changed 7 years ago by Owen Arnold
- Status changed from reopened to inprogress
refs #8755. Remove unused imports and modify import packages.
Changeset: 011f0e381e7498126f26bb75800168329df4f90e
comment:11 Changed 7 years ago by Owen Arnold
- Status changed from inprogress to verify
- Resolution set to fixed
comment:13 Changed 7 years ago by Jay Rainey
- Status changed from verifying to reopened
- Resolution fixed deleted
- Blocking 8764, 8765 removed
When I try to run the GUI I get another (different) error, and the GUI does not open. The error is:
ImportError: No module named settings at line 6 in '<Interface>' caused by line 5 in '~/dev/testing/Code/Mantid/scripts/Interface/ui/reflectometer/refl_gui.py' caused by line 10 in '~/dev/testing/Code/Mantid/scripts/Interface/ui/reflectometer/refl_save.py'
I believe this is is to do with the change introduced here. The GUI ran correctly when I changed the import statement from:
from settings import MissingSettings, Settings
to
from isis_reflectometry.settings import *
comment:16 Changed 7 years ago by Owen Arnold
- Status changed from reopened to inprogress
refs #8755. Fix bad import.
Changeset: 0e89495b3fb0661b8fe8eab95a21f3c393365492
comment:17 Changed 7 years ago by Owen Arnold
- Status changed from inprogress to verify
- Resolution set to fixed
Thanks for pointing this out. Not sure why it didn't fall over on the import on my dev environment. Should be fixed now.
comment:19 Changed 7 years ago by Owen Arnold
- Blocking 8765 added
comment:20 Changed 7 years ago by Jay Rainey
- Status changed from verifying to closed
I verified this ticket on Windows 7 using the testing documentation. I tested a previous release (to see how it functioned before the changes), and built the source in the ticket. All functionality works correctly.
I performed a code inspection and all the imports/renames/removals appear to be correct.
I did encounter several issues with the GUI, which are not related to this ticket. They were:
- It would be nice to be able to provide an investigation ID without an instrument.
- If I process data the workspaces are displayed on the left-hand side of the GUI. If I then delete the workspace while the GUI is open it is not deleted from the reflectometry GUI automatically.
- It was not possible to search for certain RB numbers. In some instances, RB numbers do not exist for an investigation. These are calibration collections where there is no experiment number/proposal. Instead, a string is used, such as: CAL_INTER_2009-11-23T21:44:46. This is the case across all instruments.
- When I load an investigation from the archives into the interface only the investigation number is transfered. Is it possible to extract the related fields (e.g. Angle, qmin etc) from the file to populate the table with or is this intended for the user to do manually?
- It would be useful to have a a .tbl and related .nxs files uploaded alongside the testing documentation.
comment:21 Changed 7 years ago by Jay Rainey
Merge remote-tracking branch 'origin/feature/8755_remove_dead_code'
Full changeset: f88aaec814171ba589aeb6cd0a52ba00024d8d90
comment:22 Changed 7 years ago by Keith Brown
I have explained the following:
The gui in question was just being fixed up and a few improvements made, and only those that the scientists asked for. Non-essential tweaks are being left until we make the new gui.
Regarding (1), that would quadruple the search times and is ultimately not useful to the scientists who work with one instrument at a time in this gui. There is no point returning results for CRISP, POLREF and INTER when they're just looking for SURF runs and intend to run quick on those SURF runs.
As for (2) the way the gui is built it can't happen as the interface is in python and only updates when told to (on an instrument change or RB search) so deleted workspaces won't disappear form the gui until the next refresh. I will need to check the error handling on this as I can't recall how this is handled.
(3) was news to me, as far as i knew the investigation IDs were all numerical. This will need looking at.
EDIT: I have just looked in the journals for the date mentioned in that experiment Id string and all the entries for that day have 0 as the Experiment ID, so that string was never in the journals to be found by this gui
comment:23 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 9599