marcomusy / vedo

A python module for scientific analysis of 3D data based on VTK and Numpy
https://vedo.embl.es
MIT License
2.03k stars 265 forks source link

allow mesh classes to handle copy.deepcopy #1022

Closed JeffreyWardman closed 8 months ago

JeffreyWardman commented 8 months ago

This PR enables the use of copy.deepcopy for meshes.

Use case:

import vedo
from pydantic import BaseModel

class Data(BaseModel):
    mesh: vedo.Mesh

data = Data(mesh.copy())

Overwriting the mesh with mesh = mesh.copy() would update the mesh to the new memory but would point to the same object for all copies of Data.

Doing the below results in the error underneath:

from copy import deepcopy
data2 = deepcopy(data)
  File "python3.10/site-packages/pydantic/main.py", line 260, in model_copy
    copied = self.__deepcopy__() if deep else self.__copy__()
  File "python3.10/site-packages/pydantic/main.py", line 711, in __deepcopy__
    _object_setattr(m, '__dict__', deepcopy(self.__dict__, memo=memo))
  File "/usr/lib/python3.10/copy.py", line 146, in deepcopy
    y = copier(x, memo)
  File "/usr/lib/python3.10/copy.py", line 231, in _deepcopy_dict
    y[deepcopy(key, memo)] = deepcopy(value, memo)
  File "/usr/lib/python3.10/copy.py", line 146, in deepcopy
    y = copier(x, memo)
  File "/usr/lib/python3.10/copy.py", line 231, in _deepcopy_dict
    y[deepcopy(key, memo)] = deepcopy(value, memo)
  File "/usr/lib/python3.10/copy.py", line 153, in deepcopy
    y = copier(memo)
  File "python3.10/site-packages/pydantic/main.py", line 711, in __deepcopy__
    _object_setattr(m, '__dict__', deepcopy(self.__dict__, memo=memo))
  File "/usr/lib/python3.10/copy.py", line 146, in deepcopy
    y = copier(x, memo)
  File "/usr/lib/python3.10/copy.py", line 231, in _deepcopy_dict
    y[deepcopy(key, memo)] = deepcopy(value, memo)
  File "/usr/lib/python3.10/copy.py", line 172, in deepcopy
    y = _reconstruct(x, memo, *rv)
  File "/usr/lib/python3.10/copy.py", line 271, in _reconstruct
    state = deepcopy(state, memo)
  File "/usr/lib/python3.10/copy.py", line 146, in deepcopy
    y = copier(x, memo)
  File "/usr/lib/python3.10/copy.py", line 231, in _deepcopy_dict
    y[deepcopy(key, memo)] = deepcopy(value, memo)
  File "/usr/lib/python3.10/copy.py", line 161, in deepcopy
    rv = reductor(4)
TypeError: cannot pickle 'vtkmodules.vtkRenderingOpenGL2.vtkOpenGLPolyDataMapper' object
marcomusy commented 8 months ago

Interesting, what if we reuse the deep keyword for it:


    def __copy__(self):
        return self.clone(deep=False)

    def __deepcopy__(self, memo):
        return self.clone(deep=memo)

    def copy(self, deep=True):
        """Return a copy of the object. Alias of `clone()`."""
        return self.clone(deep=deep)

    def clone(self, deep=True):

        poly = vtk.vtkPolyData()

        if deep: # if a memo object is passed this checks as True
            poly.DeepCopy(self.dataset)
        else:
            poly.ShallowCopy(self.dataset)

        if isinstance(self, vedo.Mesh):
            cloned = vedo.Mesh(poly)
        else:
            cloned = Points(poly)

        cloned.transform = self.transform.clone()
        cloned.copy_properties_from(self)

        cloned.name = str(self.name)
        cloned.filename = str(self.filename)
        cloned.info = dict(self.info)
        cloned.pipeline = utils.OperationNode("clone", parents=[self], shape="diamond", c="#edede9")

        if isinstance(deep, memo_class):
        # if 'memo_somthing' in deep.__class__.__name__: # or check by string
            memo = deep
            memo[id(self)] = cloned

        return cloned

this would avoid the need of exposing a potentially confusing keyword to a very commonly used method...

JeffreyWardman commented 8 months ago

I've updated to your feedback. Happy to merge if you're content with it.

But is that potentially misleading? You would have to set deep=True internally in clone to ensure a deepcopy is still made via this change. It's also allowing two very distinct data types for the variable deep.

if deep: # if a memo object is passed this checks as True
-->
if deep or isinstance(deep, dict): # if a memo object is passed this checks as True
marcomusy commented 8 months ago

But is that potentially misleading? You would have to set deep=True internally in clone to ensure a deepcopy is still made via this change.

If a user is doing something like e.g. mesh.copy(deep=1) it should have the right behavior from what I see... can you confirm

It's also allowing two very distinct data types for the variable deep.

Yes - not very elegant... but I guess it's least disrupting of the API...

JeffreyWardman commented 8 months ago

Yes deep=1 isn't a duct so will have the same behaviour as current.