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

RemoveMaskedSpectra algorithm. #678

Merged
merged 15 commits into from May 1, 2015
Merged

RemoveMaskedSpectra algorithm. #678

merged 15 commits into from May 1, 2015

Conversation

mantid-roman
Copy link
Contributor

11619

To test

  • Check that CropWorkspace and ExtractSingleSpectrum still work.
  • See the doc files for usage examples.
  • Open the instrument view with some data.
  • Go to the mask tab and select some detectors.
  • Save selection as ROI to a mask workspace.
  • Use this mask workspace in RemoveMaskedSpectra to extract spectra with the selected detectors.
  • Check that the output workspace contains only spectra from the selection.

@mantid-roman mantid-roman added the Framework Issues and pull requests related to components in the Framework label Apr 30, 2015
@mantid-roman mantid-roman added this to the Release 3.4 milestone Apr 30, 2015
@FedeMPouzols FedeMPouzols self-assigned this Apr 30, 2015
@FedeMPouzols
Copy link
Contributor

I was testing this and everything seemed ok. But we have got some merge conflict this morning. Could you take a look? I'd then double-check again and merge it.

@mantid-roman
Merge branch 'master' into 11619_crop_by_mask
349d1bd 
Conflicts:
	Code/Mantid/Framework/Algorithms/src/CropWorkspace.cpp
Re #11619.
FedeMPouzols
FedeMPouzols reviewed May 1, 2015
View reviewed changes
Code/Mantid/Framework/Algorithms/src/ExtractSpectra.cpp
if (m_inputWorkspace->axes() > 1) {
inAxis1 = m_inputWorkspace->getAxis(1);
outAxis1 = outputWorkspace->getAxis(1);
outTxtAxis = dynamic_cast<TextAxis *>(outAxis1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was already there anyway, but I suspect that coverity will complain that outTxtAxis is used without checking the dynamic_cast outcome?

@FedeMPouzols
Copy link
Contributor

This looks fine. The algorithms are well tested and everything seems to work as expected. Interactive use with the instrument viewer works very nicely. I'm just waiting for the system and doc tests to finish, but I think this is ready to be merged in. I just put a comment for a couple of typos, let me know if you want to fix those before I merge this branch. These typos don't need CI builds again.

@mantid-roman
Copy link
Contributor Author

I want to fix the dynamic_cast thing. It will start full rebuild.

FedeMPouzols added a commit that referenced this pull request May 1, 2015
@FedeMPouzols
Merge pull request #678 from mantidproject/11619_crop_by_mask
330b2ab 
RemoveMaskedSpectra algorithm.
@FedeMPouzols FedeMPouzols merged commit 330b2ab into master May 1, 2015
@FedeMPouzols FedeMPouzols deleted the 11619_crop_by_mask branch May 1, 2015 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@FedeMPouzols FedeMPouzols

Labels
Framework Issues and pull requests related to components in the Framework
Projects
None yet
Milestone
Release 3.4
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mantid-roman @FedeMPouzols