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

DGS reduction interface bug #15386

Merged
merged 3 commits into from Mar 14, 2016
Merged

DGS reduction interface bug #15386

merged 3 commits into from Mar 14, 2016

Conversation

AndreiSavici
Copy link
Member

Fix #11602

Changed how the parameters are handled, from int to string. This allows for empty inputs for the monitor spectra. Also put a try...except around the UI checking when one clicks the reduce button

To test:

  1. Open DgsReduction interface (not the algorithm), choose your facility and instrument, choose a file. By default, the GUI is populated with the default monitor spectra numbers. See that you can reduce the file.
  2. Delete the content for spectrum numbers for monitor 1 and 2. re-run the reduction. Before these changes, the script will produce an error. When the reduction button is clicked, all buttons at the bottom are disabled, until the reduction finishes. If the UI fails, that exception is not caught, and the buttons are not re-enabled. This should not happen with this fix

Release notes: None - minor bugfix

@mantid-builder
Copy link
Collaborator

Thanks for the changes. Here's a few things that we'll use to start the review process:

Main Reviewer

Please comment on the following:

Pull Request
  • Does the title summarize the changes, i.e. it's not the automatically generated one?
  • Is an issue link included/required, e.g. "Fixes #XXXX"? If there is an issue link do the changes look sensible and consistent with issue description?
  • Are there adequate testing instructions?
  • A link to the updated release notes has been provided or no release note updates were necessary?
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 class(es) in isolation from one another?
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.

Second Reviewer (@gatekeepers)

  • Have adequate tests, both success & failure cases, been performed?,
  • Has the code been reviewed?,
  • Ready to go? Hit Merge!

@AndreiSavici AndreiSavici added Bug Issues and pull requests that are regressions or would be considered a bug by users (e.g. crashing) Direct Inelastic Issues and pull requests related to direct inelastic Component: GUI labels Feb 19, 2016
@AndreiSavici AndreiSavici added this to the Release 3.7 milestone Feb 19, 2016
@lottiegreenwood
Copy link

The bug is fixed, I can try to reduce files and remove spectrum numbers for monitors without causing it to hang.
:shipit:

quantumsteve added a commit that referenced this pull request Mar 14, 2016
@quantumsteve quantumsteve merged commit 3a2c3ca into master Mar 14, 2016
@quantumsteve quantumsteve deleted the 11602_DGS_interface_bugfix branch March 14, 2016 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues and pull requests that are regressions or would be considered a bug by users (e.g. crashing) Direct Inelastic Issues and pull requests related to direct inelastic
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DGS Reduction stuck if reduce with no monitor set
4 participants