orbingol / NURBS-Python

Object-oriented pure Python B-Spline and NURBS library
https://onurraufbingol.com/NURBS-Python/
MIT License
603 stars 153 forks source link

Cannot set delta value after copying multi.SurfaceContainer() #131

Open sphh opened 3 years ago

sphh commented 3 years ago

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:

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
orbingol commented 3 years ago

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.

sphh commented 3 years ago

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