Discrepancy between API and documentation with `.observer`

[galou]

The fpo argument is missing in all function with the @{prop}.observer decorator.

Here is a patch to update the documentation but I'd rather update the API to remove fpo as argument since it can be accessed via self.Object (by the way, the documentation doesn't state that).

diff --git a/docs/documentation.md b/docs/documentation.md
index 231bed0..5c205e7 100644
--- a/docs/documentation.md
+++ b/docs/documentation.md
@@ -652,7 +652,7 @@ class CustomBoxProxy:
         fpo.Shape = Part.makeBox(self.length, self.width, self.height)

     @length.observer
-    def length_changed(self, new_value, old_value):
+    def length_changed(self, fpo, new_value, old_value):
         print(f"Hey! length has changed from {old_Value} to {new_value}")

@@ -719,7 +719,7 @@ class MyMagicProxy:

 # Optional listener
 @my_property.observer

• def my_property_obs(self, new_value, old_value):

• def my_property_obs(self, fpo, new_value, old_value): print(f"my_property has changed from {old_value} to {new_value}")

  
  @@ -809,15 +809,15 @@ class MyMagicProxy
   my_prop3 = PropertyInteger(section="Basic", default=5)
  
   @my_prop1.observer

• def listener1(self, new_value, old_value):

• def listener1(self, fpo, new_value, old_value): print(f"my_property1 has changed from {old_value} to {new_value}")

  @my_prop2.observer

• def listener2(self, new_value):

• def listener2(self, fpo, new_value): print(f"my_property2 has changed {new_value}")

  @my_prop3.observer

• def listener3(self):

• def listener3(self, fpo): print(f"my_property3 has changed")

  @@ -1640,4 +1640,4 @@ ex10.create_cube_pd()

  [!IMPORTANT] -> Copy fpo.py in only one place to avoid a name conflicts. \ No newline at end of file +> Copy fpo.py in only one place to avoid a name conflicts.

[mnesarco]

@galou Thank you.

[galou]

What about removing the fpo argument? I can try a PR if you wish.

[mnesarco]

I am not sure about removing it. It will break some of my existing code that depends on fpo. It is apparently redundant and it is inconsistent with the observers in the ViewProxy API. Let me evaluate the change for a few days before taking action.

[galou]

If removal, then I'd would remove it from all on_* callbacks (also in view_proxy naturally). I actually never understood why the object argument exists in callbacks in the FreeCAD API.

I understand that a lot of changes may be implied but a good regex or some AI should solve this (I can even assist in this). From my point of view this is worth it.

Are you aware of workbenches using this great API? I just started and love it. It makes writing workbenches much more pleasant. Before that, I also used a base class for proxies but the functionalities were limited to ensuring that all required properties are defined in execute().

[mnesarco]

This API is not official and to be honest, there is not much interest on it from the core FreeCAD teams right now as they are very busy with FC 1.0 Release. So I don't think any existing workbench is using it right now. I started creating this API for my own internal Workbenches but finally decided to document it and publish, during the process i redesigned it almost entirely :D. My main goal is to make it consistent and documented.

I am glad you are interested, i have no problem introducing changes to make it better. I can keep my internal projects isolated from this public version.

I paused the work on this until FC 1.0 release because current FC dev version is very unstable. The base FC API is already very inconsistent, incomplete and mostly undocumented or the documentation is very outdated, so i try to get the real info from the FC source code itself and document by myself.

I also have another API to simplify the GUI code. It was initially released as fcscript but at the end i am going to merge both projects. Also the fcscrip ui part was highly updated during my overhaull of Marz Workbech but the updated version is still used only in Marz workbench itself.

[mnesarco]

What about removing the fpo argument? I can try a PR if you wish.

I have been thinking about this, and I think that is is not a good idea to relay on self.Object. As a general rule of thumbs it is better to avoid direct state access from extension APIs. I know that it is not 100% consistent right now, but it is always better to provide access through methods or explicit arguments rather than direct object attributes, specially when the extension is based on meta-programming and even in inheritance based extensions.

Direct attribute access to framework provided state is always risky and limits the flexibility of new unexpected changes, while methods can maintain backwards compatibility for longer periods and explicit method arguments in callbacks are more simple in many cases (not always but mostly).

As the FreeCAD python API is not exactly a well designed and consistent API, i want to keep the maximum flexibility possible to support unexpected limitations in the future.

Passing the FPO instance to the callback/listener makes it easy to delegate to external components without access to the proxy class and makes future changes easier, specially if more meta-programming is required at some point.