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

Allow overwriting members of a WorkspaceGroup when setting output properties to the same workspace name #828

Merged
merged 6 commits into from
Jun 8, 2015
Merged

Allow overwriting members of a WorkspaceGroup when setting output properties to the same workspace name #828

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7a15689
Add the ability to overwrite members of a group
DanNixon Jun 1, 2015
05ddb2d
Clang format algorithm Refs #11879
DanNixon Jun 1, 2015
3f4cd7b
Merge branch 'master' into 11879_workspacegroup_algorithm_overwrite
DanNixon Jun 4, 2015
3d5d408
Small tidy up Re #12717
DanNixon Jun 8, 2015
0af002c
Remove duplicate logic Re #11879
DanNixon Jun 8, 2015
84a900d
Correct type Re #11879
DanNixon Jun 8, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 29 additions & 1 deletion Code/Mantid/Framework/API/src/Algorithm.cpp
View file
Open in desktop
Expand Up @@ -39,6 +39,20 @@ namespace API {
namespace {
/// Separator for workspace types in workspaceMethodOnTypes member
const std::string WORKSPACE_TYPES_SEPARATOR = ";";

class WorkspacePropertyValueIs {
public:
WorkspacePropertyValueIs(std::string value) : m_value(value){};
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I didn't see it earlier but value should be passed by const reference.

bool operator()(IWorkspaceProperty *property) {
Property *prop = dynamic_cast<Property *>(property);
if (!prop)
return false;
return prop->value() == m_value;
}

private:
std::string m_value;
};
}

// Doxygen can't handle member specialization at the moment:
Expand Down Expand Up @@ -1324,8 +1338,22 @@ bool Algorithm::processGroups() {
dynamic_cast<Property *>(m_pureOutputWorkspaceProps[owp])) {
// Default name = "in1_in2_out"
std::string outName = outputBaseName + "_" + prop->value();

WorkspacePropertyValueIs comp(prop->value());
auto inputProp = std::find_if(m_inputWorkspaceProps.begin(),
m_inputWorkspaceProps.end(), comp);

// Overwrite workspaces in any input property if they have the same
// name as an output (i.e. copy name button in algorithm dialog used)
// (only need to do this for a single input, multiple will be handled
// by ADS)
if (inputProp != m_inputWorkspaceProps.end() &&
dynamic_cast<Property *>(*inputProp)->value() == prop->value())
outName = m_groups[inputProp - m_inputWorkspaceProps.begin()][entry]
->name();

// Except if all inputs had similar names, then the name is "out_1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it necessary to create inputProp variable and do the second if? It looks to me that the second if's condition is always true when the first if is true.

Copy link
Member Author

Choose a reason for hiding this comment

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

@mantid-roman It protects against the dynamic_cast failing, although there shouldn't really be a case where it does.

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant inputProperty. It is only used to repeat the checks done in WorkspacePropertyValueIs.

Copy link
Member Author

Choose a reason for hiding this comment

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

I see what you were meaning now, I could remove the first if.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't if (inputProp != m_inputWorkspaceProps.end()) be enough here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. I have no idea what I was previously thinking.

if (m_groupsHaveSimilarNames)
else if (m_groupsHaveSimilarNames)
outName = prop->value() + "_" + Strings::toString(entry + 1);

// Set in the output
Expand Down
48 changes: 48 additions & 0 deletions Code/Mantid/Framework/API/test/AlgorithmTest.h
View file
Open in desktop
Expand Up @@ -770,6 +770,54 @@ class AlgorithmTest : public CxxTest::TestSuite
}
}

/// Rewrite first input group
void test_processGroups_rewriteFirstGroup()
{
Mantid::API::AnalysisDataService::Instance().clear();
WorkspaceGroup_sptr group = do_test_groups("D", "D1,D2,D3",
"B", "B1,B2,B3", "C", "C1,C2,C3");

TS_ASSERT_EQUALS( ws1->name(), "D1" );
TS_ASSERT_EQUALS( ws1->getTitle(), "D1+B1+C1" );
TS_ASSERT_EQUALS( ws1->readY(0)[0], 234 );
TS_ASSERT_EQUALS( ws2->name(), "D2" );
TS_ASSERT_EQUALS( ws2->getTitle(), "D2+B2+C2" );
TS_ASSERT_EQUALS( ws3->name(), "D3" );
TS_ASSERT_EQUALS( ws3->getTitle(), "D3+B3+C3" );
}

/// Rewrite second group
void test_processGroups_rewriteSecondGroup()
{
Mantid::API::AnalysisDataService::Instance().clear();
WorkspaceGroup_sptr group = do_test_groups("A", "A1,A2,A3",
"D", "D1,D2,D3", "C", "C1,C2,C3");

TS_ASSERT_EQUALS( ws1->name(), "D1" );
TS_ASSERT_EQUALS( ws1->getTitle(), "A1+D1+C1" );
TS_ASSERT_EQUALS( ws1->readY(0)[0], 234 );
TS_ASSERT_EQUALS( ws2->name(), "D2" );
TS_ASSERT_EQUALS( ws2->getTitle(), "A2+D2+C2" );
TS_ASSERT_EQUALS( ws3->name(), "D3" );
TS_ASSERT_EQUALS( ws3->getTitle(), "A3+D3+C3" );
}

/// Rewrite multiple group
void test_processGroups_rewriteMultipleGroup()
{
Mantid::API::AnalysisDataService::Instance().clear();
WorkspaceGroup_sptr group = do_test_groups("A", "A1,A2,A3",
"D", "D1,D2,D3", "D", "D1,D2,D3");

TS_ASSERT_EQUALS( ws1->name(), "D1" );
TS_ASSERT_EQUALS( ws1->getTitle(), "A1+D1+D1" );
TS_ASSERT_EQUALS( ws1->readY(0)[0], 234 );
TS_ASSERT_EQUALS( ws2->name(), "D2" );
TS_ASSERT_EQUALS( ws2->getTitle(), "A2+D2+D2" );
TS_ASSERT_EQUALS( ws3->name(), "D3" );
TS_ASSERT_EQUALS( ws3->getTitle(), "A3+D3+D3" );
}

private:
IAlgorithm_sptr runFromString(const std::string & input)
{
Expand Down