NURBS-Python icon indicating copy to clipboard operation
NURBS-Python copied to clipboard

Cannot set delta value after copying multi.SurfaceContainer()

Open sphh opened this issue 4 years ago • 2 comments

Describe the bug I have a multi.SurfaceContainer with two surfaces. After making a copy with copy.deepcopy() I cannot set the delta value.

To Reproduce Steps to reproduce the behavior:

>>> from geomdl import multi
>>> from geomdl.shapes import surface
>>> import copy
>>> s0 = surface.cylinder(1, 1)
>>> s1 = surface.cylinder(1, 2)
>>> m = multi.SurfaceContainer(s0, s1)
>>> m1 = copy.deepcopy(m)
>>> m1.delta = 0.1
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.8/dist-packages/geomdl/multi.py", line 204, in delta
    self.reset()
  File "/usr/local/lib/python3.8/dist-packages/geomdl/multi.py", line 716, in reset
    super(SurfaceContainer, self).reset()
  File "/usr/local/lib/python3.8/dist-packages/geomdl/multi.py", line 304, in reset
    self._cache['evalpts'][:] = []
KeyError: 'evalpts'

Expected Behavior Not throwing any error.

Configuration:

  • geomdl install source: PyPI
  • geomdl version/branch: 5.3.1

Additional Details (Optional)

My solution is (multi.py):

class AbstractContainer(...):

    def reset(self):
        """ Resets the cache. """
        try:
            self._cache['evalpts'][:] = []
        except KeyError:
            pass

class SurfaceContainer(...):

    def reset(self):
        """ Resets the cache. """
        super(SurfaceContainer, self).reset()
        try:
            self._cache['vertices'][:] = []
        except KeyError:
            pass
        try:
            self._cache['faces'][:] = []
        except KeyError:
            pass

sphh avatar Jun 09 '21 19:06 sphh

Thanks for the bug report.

copy.copy and copy.deepcopy won't copy the caches (defined in abstract.GeomdlBase) but it seems it won't reinitialize the caches too. I think removing [:] should work fine for now from the reset method of the base and surface container classes. I'll also need to port this to geomdl 6.x if I haven't fixed it already.

Alternatively, it could be possible to move the cache functionality to __new__ magic method as the parent class calls __new__ during copy and deepcopy. This might be a better solution but I'll check it thoroughly and make sure that it won't break anything.

I'll try to push an update this weekend.

orbingol avatar Jun 09 '21 19:06 orbingol

I tried your approach and removed [:]. That also works for me. Thanks!

sphh avatar Jun 09 '21 20:06 sphh