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.

Sign up for GitHub

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

Jump to bottom

PeakIntensityVsRadius throws an error if parameter is < 0 #8394

Closed
mantid-builder opened this issue Jul 29, 2013 · 3 comments · Fixed by #20053
Closed

PeakIntensityVsRadius throws an error if parameter is < 0 #8394

mantid-builder opened this issue Jul 29, 2013 · 3 comments · Fixed by #20053
Assignees
@edwardb96
Labels
Framework Issues and pull requests related to components in the Framework Induction Straightforward, low risk issues suitable for new starters

Comments

@mantid-builder
Copy link
Collaborator

This issue was originally TRAC 7549

Original Reporter: Samuel Jackson

This should be an easy fix. The NumSteps parameter in PeakIntensityVsRadius throws the following error when the NumSteps is less than 0:

Error in execution of algorithm PeakIntensityVsRadius:
vector::_M_default_append

My guess would be that adding a bounded validator would fix this no problem.
@mantid-builder
Copy link
Collaborator Author

@NickDraper (2014-02-14T11:05:00):
Bulk move to assigned at the introduction of the triage step

@mantid-builder mantid-builder added the Framework Issues and pull requests related to components in the Framework label Jun 3, 2015
@NickDraper
Copy link
Contributor

  1. add failing test
  2. fix issue
  3. test should pass

@edwardb96 edwardb96 self-assigned this Jul 17, 2017
gemmaguest added a commit that referenced this issue Jul 17, 2017
@gemmaguest
Refs #8394 Added test and validator creation.
f556671 
Added commented out BoundedValidator creation to NumSteps property of PeakIntensityVsRadius.
gemmaguest added a commit that referenced this issue Jul 17, 2017
@gemmaguest
Refs #8394 Added failing unit test.
d2a3b59 
Added new methods, doTestThrowsForInvalid and doTestNoThrowForValid to
make unit tests clearer.

Added extra test which ensures that validateInput throws when passed a
negative value for the NumSteps property.
@gemmaguest
Copy link
Member

Note that the commits here are Edward's work, he was just using my machine!

edwardb96 pushed a commit that referenced this issue Jul 20, 2017
Re #8394 Removed assertion duplication.
6d2c93a 
Many assertions were repeated in two of the PeakIntensityVsRadiusTests. They have been extracted into macros which broadly describe what they do.
edwardb96 pushed a commit that referenced this issue Jul 20, 2017
Re #8394 Removed further assertion duplication.
9cbd103 
- Removed duplicated ASSERT_THROWS_NOTHINGs for algorithm property setters.
- Removed duplicated ASSERT_THROWS_NOTHING and ASSERT for initialization of the algorithm object.
edwardb96 pushed a commit that referenced this issue Jul 20, 2017
Re #8394 Contined Refactoring and Added Failing Test.
8370465 
- Continued to refactor and eliminate duplication from PeakIntensityVsRadius unit tests.
- Converted added assertion macros to proper functions.
- Added failing unit test to prove lack of validator.
edwardb96 pushed a commit that referenced this issue Jul 20, 2017
Re #8394: Moved assertions.
320630b 
- Assertions are moved closer to the test methods they are used in.
@edwardb96 edwardb96 mentioned this issue Jul 20, 2017
Merged
7 tasks
edwardb96 pushed a commit that referenced this issue Jul 20, 2017
Re #8394: Uncommented fix, applied clang-format.
85234d1 
- Applied clang format manually to PeakIntensityVsRadius.
- Uncommented fix for issue.
edwardb96 pushed a commit that referenced this issue Jul 20, 2017
Re #8394 Fix formatting error.
10c32af
edwardb96 pushed a commit that referenced this issue Jul 24, 2017
Re #8394 Remove Unused NumSteps parameter from test helpers.
fcc39b4 
- Removed the unused NumSteps parameter from the PeakIntensityVsRadius test helpers.
- Removed the unused m_alg member variable from the PeakIntensityVsRadius suite class.
edwardb96 pushed a commit that referenced this issue Jul 24, 2017
Re #8394: Removed additional type specifier.
b684506 
- The DEFAULT_NUM_STEPS constant had both auto and int type specifiers this commit removes the int.
@DanielMurphy22 DanielMurphy22 added Induction Straightforward, low risk issues suitable for new starters and removed Induction 1 labels Nov 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@edwardb96 edwardb96

Labels
Framework Issues and pull requests related to components in the Framework Induction Straightforward, low risk issues suitable for new starters
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add Validator to NumSteps in PeakintensityVsRadius mantidproject/mantid
5 participants
@NickDraper @mantid-builder @edwardb96 @gemmaguest @DanielMurphy22