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

Create clone functionality for MatrixWorkspaces #12687

Closed
AntonPiccardoSelg opened this issue May 27, 2015 · 0 comments
Closed

Create clone functionality for MatrixWorkspaces #12687

AntonPiccardoSelg opened this issue May 27, 2015 · 0 comments
Assignees
@SimonHeybrock
Labels
Framework Issues and pull requests related to components in the Framework
Milestone
Release 3.5

Comments

@AntonPiccardoSelg
Copy link
Contributor

This issue was originally TRAC 11849

Matrix workspaces are quite frequently cloned within algorithms. Currently this requires to setup a child algorithm and executing it.

It would be good to have a clone() method for MatrixWorkspaces which returns a deep copy of the current object.

Keywords: Workspace-structure
@AntonPiccardoSelg AntonPiccardoSelg added the Framework Issues and pull requests related to components in the Framework label Jun 3, 2015
@AntonPiccardoSelg AntonPiccardoSelg added this to the Release 3.5 milestone Jun 3, 2015
@SimonHeybrock SimonHeybrock self-assigned this Jun 23, 2015
SimonHeybrock added a commit that referenced this issue Jun 23, 2015
@SimonHeybrock
Re #12687 Proposal for a clone interface for Workspace.
c99219b
SimonHeybrock added a commit that referenced this issue Jun 24, 2015
@SimonHeybrock
Re #12687 Typo in MatrixWorkspace::doClone().
1f077ec
This was referenced Jun 24, 2015
Closed
Closed
SimonHeybrock added a commit that referenced this issue Jun 26, 2015
@SimonHeybrock
Re #12687 Moved TableWorkspace clone code to copy constructor.
aabb7aa
SimonHeybrock added a commit that referenced this issue Jun 26, 2015
@SimonHeybrock
Re #12687 Added clone() to Workspaces. Retaining old behavior.
3198c0d 
Added non-virtual clone() interface with a virtual private doClone()
method to Workspace and derived classes. Workspaces that had a clone()
method before were adapted to have a common interface everywhere. In
cases where there was no clone() method previously we simply throw an
error for now, until this new functionality is implemented.

Some tests/mocks had to be adapted since there there is a pure virtual
doClone() in all abstract classes.

Do to smart pointer incompatibilities (mainly due to old boost versions)
we had do extract the raw pointer from the unique_ptr returned by clone
and put it into a boost::shared_ptr manually, in some cases. See also
issue 12949.
SimonHeybrock added a commit that referenced this issue Jun 26, 2015
@SimonHeybrock
Re #12687 Removed override keyword from pure virtual methods.
6e8b718 
This seems to be incompatible with the RHEL6 compiler.
SimonHeybrock added a commit that referenced this issue Jun 29, 2015
@SimonHeybrock
Re #12687 Added doClone() to mock workspace.
e179294
SimonHeybrock added a commit that referenced this issue Jun 29, 2015
@SimonHeybrock
Re #12687 Added doClone() to mock workspace.
910d35d
SimonHeybrock added a commit that referenced this issue Jun 29, 2015
@SimonHeybrock
Re #12687 Workspace::clone() fixes for old boost w/o unique_ptr.
e30c257 
Boost < 1.53 does not support construction from a std::unique_ptr, so
for compatibility we must call clone().release() when dropping it to a
boost::shared_ptr.
SimonHeybrock added a commit that referenced this issue Jun 30, 2015
@SimonHeybrock
Re #12687 Moved TableWorkspace clone code to copy constructor.
aa45f08
SimonHeybrock added a commit that referenced this issue Jun 30, 2015
@SimonHeybrock
Re #12687 Added clone() to Workspaces. Retaining old behavior.
3231d87 
Added non-virtual clone() interface with a virtual private doClone()
method to Workspace and derived classes. Workspaces that had a clone()
method before were adapted to have a common interface everywhere. In
cases where there was no clone() method previously we simply throw an
error for now, until this new functionality is implemented.

Some tests/mocks had to be adapted since there there is a pure virtual
doClone() in all abstract classes.

Do to smart pointer incompatibilities (mainly due to old boost versions)
we had do extract the raw pointer from the unique_ptr returned by clone
and put it into a boost::shared_ptr manually, in some cases. See also
issue 12949.
SimonHeybrock added a commit that referenced this issue Jun 30, 2015
@SimonHeybrock
Re #12687 Removed override keyword from pure virtual methods.
bfe6047 
This seems to be incompatible with the RHEL6 compiler.
SimonHeybrock added a commit that referenced this issue Jun 30, 2015
@SimonHeybrock
Re #12687 Added doClone() to mock workspace.
88769fe
SimonHeybrock added a commit that referenced this issue Jun 30, 2015
@SimonHeybrock
Re #12687 Workspace::clone() fixes for old boost w/o unique_ptr.
4838e45 
Boost < 1.53 does not support construction from a std::unique_ptr, so
for compatibility we must call clone().release() when dropping it to a
boost::shared_ptr.
SimonHeybrock added a commit that referenced this issue Jun 30, 2015
@SimonHeybrock
Re #12687 Fixed potentially wrong TableWorkspace copy constructor.
ec90b59 
The previous code which derives from the old implementation of clone()
did not call the base class copy constructor but the base class
constructor. This seemed wrong (though I am not aware of any related
issues).
SimonHeybrock added a commit that referenced this issue Jun 30, 2015
@SimonHeybrock
Re #12687 clone() for MementoTableWorkspace.
158a00b 
Previously (before 12687) there was no implementation of clone(). That
is, when calling clone() though a base class (e.g., ITableWorkspace) we
would use TableWorkspace::clone() on an object of type
MementoTableWorkspace. Was that doing the right thing?
SimonHeybrock added a commit that referenced this issue Jun 30, 2015
@SimonHeybrock
Re #12687 clone() for SplittersWorkspace.
48215c9 
Previously (before 12687) there was no implementation of clone().
That is, when calling clone() though a base class (e.g., ITableWorkspace)
we would use TableWorkspace::clone() on an object of type
SplittersWorkspace. Was that doing the right thing?
SimonHeybrock added a commit that referenced this issue Jun 30, 2015
@SimonHeybrock
Re #12687 clone() implementation for MDHistoWorkspace.
712a8b4
SimonHeybrock added a commit that referenced this issue Jun 30, 2015
@SimonHeybrock
Re #12687 Cleanup in CloneWorkspace.
b3927f9
SimonHeybrock added a commit that referenced this issue Jun 30, 2015
@SimonHeybrock
Re #12687 clone() implementation for MDEventWorkspace.
e281689 
Note that this does not (yet?) support cloning a file-backed workspace
(see CloneMDWorkspace algorithm).
SimonHeybrock added a commit that referenced this issue Jun 30, 2015
@SimonHeybrock
Re #12687 clone() implementation for WorkspaceSingleValue.
bfc4bae
SimonHeybrock added a commit that referenced this issue Jul 2, 2015
@SimonHeybrock
Re #12687 Fixed incomplete copy constructor of MatrixWorkspace.
864a6ed
SimonHeybrock added a commit that referenced this issue Jul 2, 2015
@SimonHeybrock
Re #12687 clone() implementation for MatrixWorkspaces (not Event).
38eb3ee
SimonHeybrock added a commit that referenced this issue Jul 2, 2015
@SimonHeybrock
Re #12687 clone() implementation for EventWorkspace.
c4c389e
SimonHeybrock added a commit that referenced this issue Jul 2, 2015
@SimonHeybrock
Re #12687 Unified code in CloneWorkspace algorithm.
85873e2
SimonHeybrock added a commit that referenced this issue Jul 2, 2015
@SimonHeybrock
Re #12687 Cleanup/unification in CloneWorkspace algorithm.
3deb7d1 
Using the polymorpic clone, looking at the inheritance structure, code
in CloneWorkspace could be unified. PeaksWorkspace is a child of
ITableWorkspace, so Peaks* and Table* cast could be combined.
EventWorkspace is a child of MAtrixWorkpace, so they could also be
combined.
The remaining special case is due to MDEventWorkspace, which still has
some special code on CloneMDWorkspace.
SimonHeybrock added a commit that referenced this issue Jul 2, 2015
@SimonHeybrock
Re #12687 Troubling pure virtual function with diamond inheritance.
68e1d02 
Workaround, see comments in code.
SimonHeybrock added a commit that referenced this issue Jul 2, 2015
@SimonHeybrock
Re #12687 Fix conflicting DLL export definitions.
3427d5f
@SimonHeybrock SimonHeybrock mentioned this issue Jul 2, 2015
Merged
SimonHeybrock added a commit that referenced this issue Jul 2, 2015
@SimonHeybrock
Re #12687 Troubling pure virtual function with diamond inheritance.
b3ea139 
Workaround, see comments in code.
SimonHeybrock added a commit that referenced this issue Jul 3, 2015
@SimonHeybrock
Re #12687 ISplittersWorkspace is not a Workspace anymore.
a941ef3 
The reason for this change is related to the diamond inheritance that we
had previously for SplittersWorkspace. VS2012 did not manage to compile
this in combination with the virtual doClone() function with covariant
return types.
Just like IMaskWorkspace, ISplittersWorkspace is now not a Workspace
anymore, and thus the diamond is gone.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@SimonHeybrock SimonHeybrock

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

No branches or pull requests
2 participants
@AntonPiccardoSelg @SimonHeybrock