Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug in reflectometry GUI #18865

Merged
merged 2 commits into from Feb 16, 2017

Conversation

raquelalvarezbanos
Copy link
Contributor

Fixes a bug when no transmission runs provided.

To test:

  • Open the old reflectometry GUI
  • Select INTER and enter run 13460, hit process. The run should be reduced and workspaces 13460_IvsQ, 13460_IvsLam and TOF (this one should be a group with a single entry) should be created in the ADS.
  • Reduce a multi-period dataset: select POLREF, enter run 18564 and hit Process, no errors should be shown and the run should be reduced.
  • As an extra check, compare results to the new interface.

Fixes #16925.

No need to update release notes, this issue was reported by a developer and picked by squish test.


Reviewer

Please comment on the following (full description):

Code Review
  • Is the code of an acceptable quality?
  • Does the code conform to the coding standards? Is it well structured with small focussed classes/methods/functions?
  • Are there unit/system tests in place? Are the unit tests small and test the a class in isolation?
  • If there are changes in the release notes then do they describe the changes appropriately?
Functional Tests
  • Do changes function as described? Add comments below that describe the tests performed?

  • How do the changes handle unexpected situations, e.g. bad input?

  • Has the relevant documentation been added/updated?

  • Is user-facing documentation written in a user-friendly manner?

  • Has developer documentation been updated if required?

  • Does everything look good? Comment with the ship it emoji but don't merge. A member of @mantidproject/gatekeepers will take care of it.

@raquelalvarezbanos raquelalvarezbanos added Component: GUI Reflectometry Issues and pull requests related to reflectometry Patch Candidate Urgent issues that must be included in a patch following a release High Priority An issue or pull request that if not addressed is severe enough to postponse a release. labels Feb 15, 2017
@raquelalvarezbanos raquelalvarezbanos added this to the Release 3.10 milestone Feb 15, 2017
@PranavBahuguna
Copy link
Contributor

Code looks fine. I ran the testing instructions - while both 13460 and 18564 could be reduced, I noticed that reducing 18564 would produce the following workspaces:

(Group) 18564
-- 18564_1
-- 18564_2
(Group) 18564_IvsLam
-- 18564_IvsLam_1
-- 18564_IvsLam_2
(Group) 18564_IvsQ
-- 18564_IvsQ_1
-- 18564_IvsQ_2
(Group) 18564_IvsQ_binned
-- 18564_IvsQ_1
-- 18564_IvsQ_2
18564_IvsQ_binned_1
18564_IvsQ_binned_2

The IvsQ and IvsQ_binned groups hold the same IvsQ workspaces and the workspaces that are supposed to be in the IvsQ_binned group are left outside. This appears to happen for any runs processed under POLREF.

@PranavBahuguna
Copy link
Contributor

PranavBahuguna commented Feb 15, 2017

Actually, the above has nothing to do with your changes.

@raquelalvarezbanos
Copy link
Contributor Author

Yes, I think it is unrelated to these changes, but given that you found it, I'd like to solve it as part of this PR, so if you could remove the squirrel I'll fix it.

@raquelalvarezbanos
Copy link
Contributor Author

@PranavBahuguna could you test this again?

@PranavBahuguna
Copy link
Contributor

Alright, workspaces are outputted correctly now.

:shipit: once checks pass

@NickDraper NickDraper merged commit f46a26a into master Feb 16, 2017
@NickDraper NickDraper deleted the 16925_Fix_reduction_with_no_transmission_run branch February 16, 2017 13:49
martyngigg pushed a commit that referenced this pull request Feb 23, 2017
Re #16925 Fix the bug

(cherry picked from commit 7758a96)

Re #16925 Fix another issue found during testing

(cherry picked from commit 16c4bbc)

Add patch release note
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
High Priority An issue or pull request that if not addressed is severe enough to postponse a release. Patch Candidate Urgent issues that must be included in a patch following a release Reflectometry Issues and pull requests related to reflectometry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants