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

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

Conversation

DanNixon
Copy link
Member

@DanNixon DanNixon commented Jun 1, 2015

Fixes #12717

This adds the ability for an algorithm to overwrite the members of a WorkspaceGroup when processing workspace groups when an output property is given the same name as an input property, before this would have created new workspaces with name_n style names, which is inconsistent with what you would expect given how algorithms run over single workspaces.

To test:

  • Code review
  • Try some algorithms that take multiple input and output workspaces and see that it behaves as expected

@DanNixon DanNixon added Framework Issues and pull requests related to components in the Framework Extra Attention Testers and Gate keepers should pay extra attention as this affects core aspects. labels Jun 1, 2015
@DanNixon DanNixon added this to the Release 3.5 milestone Jun 1, 2015
@mantid-roman mantid-roman self-assigned this Jun 5, 2015
if (inputProperty && inputProperty->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.

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.

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.


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.

mantid-roman added a commit that referenced this pull request Jun 8, 2015
…ithm_overwrite

Allow overwriting members of a WorkspaceGroup when setting output properties to the same workspace name
@mantid-roman mantid-roman merged commit 78a164c into master Jun 8, 2015
@mantid-roman mantid-roman deleted the 11879_workspacegroup_algorithm_overwrite branch June 8, 2015 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Extra Attention Testers and Gate keepers should pay extra attention as this affects core aspects. Framework Issues and pull requests related to components in the Framework
Projects
None yet
2 participants