Repository: heitzmann/gdstk (language: python)

Issue:
Reference.delete_gds_property() does not seem to actually remove the legacy GDSII style property
I'm brand new to python (3.12) and gdstk (0.9.56 I think), but I think this is a bug.  I'm trying to use gdstk to read in an OASIS file that contains GDSII style numbered properties and convert them to OASIS style named properties.  I am running into a problem when trying to delete the GDS style numbered property.  After executing the delete_gds_property method, using get_gds_property method still returns the original string instead of None.  I also tried using the delete method multiple times (even in a while loop) but it just never deletes the property.  I am currently seeing this for Reference class, but the same bug may exist for GDS style numbered properties in the FlexPath and Polygon classes (I didn't make it that far yet).  I hope the information below is enough to root cause and fix the issue.  Happy to help however I can, but I'm a hardware engineer with nearly no C++ experience.  I mention this only because I may need some explanation if I need to do anything other than the simple "pip install --user gdstk" to test out fixes.  Apologies in advance for my software ignorance.

Below you can see an example of how this doesn't seem to work the way one would expect.
```
>>> ref
<gdstk.Reference object at 0x7f08705a0530>

>>> ref.get_gds_property(42)
'FC_filler__P6U8B_FILLER3__x11970y10320'

>>> ref.delete_gds_property(42)
<gdstk.Reference object at 0x7f08705a0530>

>>> ref.get_gds_property(42)
'FC_filler__P6U8B_FILLER3__x11970y10320'

>>> ref.delete_gds_property(42)
<gdstk.Reference object at 0x7f08705a0530>
>>> ref.delete_gds_property(42)
<gdstk.Reference object at 0x7f08705a0530>
>>> ref.delete_gds_property(42)
<gdstk.Reference object at 0x7f08705a0530>
>>> ref.delete_gds_property(42)
<gdstk.Reference object at 0x7f08705a0530>

>>> ref.get_gds_property(42)
'FC_filler__P6U8B_FILLER3__x11970y10320'
>>>
```

Below is a simple example showing a similar procedure working for the OASIS style named property methods.
```
>>> ref.get_property("instName")
[b'FC_filler__P6U8B_FILLER3__x11970y10320']

>>> ref.delete_property("instName")
<gdstk.Reference object at 0x7f08705a0530>

>>> ref.get_property("instName")
>>>
```


Discussion / hints:
I confirm that a similar issue exists for FlexPath and Polygon classes.  Plus I saved the modifed OASIS file and opened it in calibredrv to check what happened with my conversion from GDS to OASIS style properties.  In the screenshot you can see both the original GDS style property 42 and the OASIS (User) style property netName.  This confirms that the GDS style properties actually are not being deleted by the method.

<img width="427" alt="image" src="https://github.com/user-attachments/assets/bb988ac7-86be-4750-902a-41a8034b0974">

Fix patch:
diff --git a/src/property.cpp b/src/property.cpp
index 8f73b11a..95d11b5c 100644
--- a/src/property.cpp
+++ b/src/property.cpp
@@ -277,7 +277,7 @@ bool remove_gds_property(Property*& properties, uint16_t attribute) {
     }
     Property* property = properties;
     while (property->next &&
-           (!is_gds_property(property->next) || property->value->unsigned_integer != attribute))
+           (!is_gds_property(property->next) || property->next->value->unsigned_integer != attribute))
         property = property->next;
     if (property->next) {
         Property* rem = property->next;


Test patch:
diff --git a/tests/property_test.py b/tests/property_test.py
index 23e487bc..a57555ec 100644
--- a/tests/property_test.py
+++ b/tests/property_test.py
@@ -68,3 +68,72 @@ def test_properties():
             ["TWO", -2.3e-4, b"two"],
             ["Three", b"\xFF\xEE", 0],
         ]
+
+
+def test_delete_gds_property():
+    def create_props(obj) -> gdstk.Reference:
+        obj.set_gds_property(100, "bar")
+        obj.set_gds_property(101, "baz")
+        obj.set_gds_property(102, "quux")
+
+        assert obj.properties == [
+            ["S_GDS_PROPERTY", 102, b"quux\x00"],
+            ["S_GDS_PROPERTY", 101, b"baz\x00"],
+            ["S_GDS_PROPERTY", 100, b"bar\x00"],
+        ]
+
+        return obj
+
+    for obj in (
+        gdstk.FlexPath(0j, 2),
+        gdstk.Label("foo", (0, 0)),
+        gdstk.rectangle((0, 0), (1, 1)),
+        gdstk.Reference("foo"),
+        gdstk.RobustPath((0.5, 50), 0),
+    ):
+        create_props(obj)
+        obj.delete_gds_property(102)
+
+        assert obj.get_gds_property(100) == "bar"
+        assert obj.get_gds_property(101) == "baz"
+        assert obj.get_gds_property(102) is None
+        assert obj.properties == [
+            ["S_GDS_PROPERTY", 101, b"baz\x00"],
+            ["S_GDS_PROPERTY", 100, b"bar\x00"],
+        ]
+
+    for obj in (
+        gdstk.FlexPath(0j, 2),
+        gdstk.Label("foo", (0, 0)),
+        gdstk.rectangle((0, 0), (1, 1)),
+        gdstk.Reference("foo"),
+        gdstk.RobustPath((0.5, 50), 0),
+    ):
+        create_props(obj)
+        obj.delete_gds_property(101)
+
+        assert obj.get_gds_property(100) == "bar"
+        assert obj.get_gds_property(101) is None
+        assert obj.get_gds_property(102) == "quux"
+        assert obj.properties == [
+            ["S_GDS_PROPERTY", 102, b"quux\x00"],
+            ["S_GDS_PROPERTY", 100, b"bar\x00"],
+        ]
+
+    for obj in (
+        gdstk.FlexPath(0j, 2),
+        gdstk.Label("foo", (0, 0)),
+        gdstk.rectangle((0, 0), (1, 1)),
+        gdstk.Reference("foo"),
+        gdstk.RobustPath((0.5, 50), 0),
+    ):
+        create_props(obj)
+        obj.delete_gds_property(100)
+
+        assert obj.get_gds_property(100) is None
+        assert obj.get_gds_property(101) == "baz"
+        assert obj.get_gds_property(102) == "quux"
+        assert obj.properties == [
+            ["S_GDS_PROPERTY", 102, b"quux\x00"],
+            ["S_GDS_PROPERTY", 101, b"baz\x00"],
+        ]