Ticket #8755 (closed: fixed)

Opened 7 years ago

Last modified 5 years ago

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:1 Changed 7 years ago by Owen Arnold

  • Status changed from new to inprogress

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

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.

Last edited 7 years ago by Jay Rainey (previous) (diff)

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:12 Changed 7 years ago by Jay Rainey

  • Status changed from verify to verifying

comment:13 Changed 7 years ago by Owen Arnold

  • Blocking 8764 added

comment:14 Changed 7 years ago by Owen Arnold

  • Blocking 8765 added

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 *

Last edited 7 years ago by Jay Rainey (previous) (diff)

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:18 Changed 7 years ago by Jay Rainey

  • Status changed from verify to verifying

comment:19 Changed 7 years ago by Owen Arnold

  • Blocking 8765 added

(In #8765) Waiting on #8755 to pass testing as will need to rebase these changes ontop of master before closing.

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:

  1. It would be nice to be able to provide an investigation ID without an instrument.
  2. 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.
  3. 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.
  4. 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?
  5. It would be useful to have a a .tbl and related .nxs files uploaded alongside the testing documentation.
Last edited 7 years ago by Jay Rainey (previous) (diff)

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

Last edited 7 years ago by Keith Brown (previous) (diff)

comment:23 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 9599

Note: See TracTickets for help on using tickets.