FreeCADView Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0001268 | FreeCAD | Bug | public | 2013-10-07 14:15 | 2016-11-07 21:50 |
| Reporter | pkoning | Assigned To | |||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | closed | Resolution | fixed | ||
| Product Version | 0.13 | ||||
| Summary | 0001268: FreeCAD python objects behave in many unPythonic ways | ||||
| Description | I'm not sure whether to call this a feature request or a bug. For one thing, it's not a single issue, it's widespread. The Python machinery of FreeCAD in many ways does things that are very surprising by Python conventions. While it's possible to work around this, it makes the code look very strange and makes for a lot more work. The general feel is of a C based API expressed in Python syntax. A couple of examples: 1. "Duck typing" is not used. Attributes that expect integers complain if the value supplied is float (or the other way around, I forgot). The Python way would be to accept anything that acts like a number. (For example, Base.Vector().z=1 fails but Base.Vector().z=1.0 is accepted.) 2. Functions require lists instead of accepting any iterable. 3. Attributes have list type but don't act like lists: attempting to change a list member has no effect. The only way to modify the attribute is to get the whole list, modify that, and set it back into the attribute. 4. Subclassing is a pain. Each object in the 3d design corresponds to 3 objects in Python (the feature object, the view object, and the shape object of the feature). This seems to explain the existence of all the "makeXyz" functions. Subclassing in such a setting is hard. I tried to subclass Arch.Floor and ended up having to make a copy of Arch.makeFloor. It would seem to make a lot more sense for each feature to be just one class, with the view and shape and feature aspects provided by multiple inheritance. Then I could just subclass the feature, with new behavior provided by overrides to __init__ and/or other methods or new methods. | ||||
| Tags | No tags attached. | ||||
| FreeCAD Information | |||||
|
|
About point 5, it is a very fundamental design decision of freecad to have those objects separated into these different components, and the advantages outweigh the inconveniences by far. Something could probably be done for most of the other points, but your bug report is too generic and unprecise. If you are interested in putting more effort in this and help to get things better, I suggest you open a separate bug report for each specific problem you encounter. |
|
|
1.) This is a limitation (or bug) in the PyCXX library we use. 2.) A concrete example for this? 3.) I guess that was a flaw due to lack of knowledge how to make a more Pythonic interface at that time. However, just changing the API here is even worse because this breaks a lot of scripts out there. Maybe this can be fixed once doing the port to Python3 which might be a major change. 4.) Indeed there are a lot of Python classes involved. The branch https://sourceforge.net/p/free-cad/code/ci/wmayer/featurepython/~/tree/ reduces the number of these Python classes. But as Yorik said you always need at least two Python classes -- one for the data stuff and one for the gui stuff. |
|
|
git show 1e86035 fixes 1.) |
|
|
Thanks! I think I can see why two objects (structure vs. view) are good to have, but 3 is a puzzle. An example of item 2: Part.makePolygon requires the first argument to be a list, it rejects a tuple as a type error. On item 3: I don't think this requires breaking scripts. I'm not suggesting anything that is currently done should be made invalid, or should have a different meaning. The request is to make things that currently don't work start to work. For example, in ArchWindow, there is a property WIndowParts which is a list of strings. I tried to change one of those (as in "win.WindowParts[3] = "120"). That doesn't work. I end up having to do this: t = win.WindowParts t[3] = "120" win.WindowParts = t which is something that no other Python module requires me to do. |
|
|
> On item 3: And that is exactly the problem. It's technically not possible to use a Python list and changing that outside of the scope of the FreeCAD object because this object won't be notified of the change. That's why an assignment is done to get this notification. So, a solution is to use a tuple instead of a list because tuples are always immutable. This however changes the API and will be even less convenient but also less confusing. |
|
|
Another option for item 3: use a subclass of list that adds notification. When assigning to the property, create an instance of that class from the supplied value. Then when assignment to an element is done, or append or extend is called (for example), the list subclass handles that. |
|
|
Sure but I don't know how much work and how complicated that would be since all methods that modify the list must be re-implemented. |
|
|
Issue 2. is solved now. |
|
|
I just attached "notifylist.py" that shows a solution for item 3 (in Python -- but the equivalent can of course also be done in C if desired). The fact that Python allows "built in" types to be subclassed makes this easy. |
|
|
|
|
|
Although 3. could maybe solved by a subscriber list, it would be confusing too. In that case lots of python object can float in the interpreter which change the document (and trigger the undo/redo framework and lots of stuff in gui). That would be very opaque. So working on copies and just assign the result is IMO the better solution! |
|
|
then we can return a tuple instead of list, to indicate that there have to be an assignment in end to change the c++ data structure. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2013-10-07 14:15 | pkoning | New Issue | |
| 2013-10-07 17:10 | yorik | Note Added: 0003720 | |
| 2013-10-08 07:25 | wmayer | Note Added: 0003721 | |
| 2013-10-08 08:54 | wmayer | Note Added: 0003722 | |
| 2013-10-08 15:25 | pkoning | Note Added: 0003723 | |
| 2013-10-09 07:06 | wmayer | Note Added: 0003724 | |
| 2013-10-09 13:06 | pkoning | Note Added: 0003725 | |
| 2013-10-09 14:36 | wmayer | Note Added: 0003726 | |
| 2013-10-11 10:06 | wmayer | Note Added: 0003727 | |
| 2013-10-11 13:37 | pkoning | File Added: notifylist.py | |
| 2013-10-11 13:39 | pkoning | Note Added: 0003732 | |
| 2013-11-04 13:31 | yorik | File Deleted: notifylist.py | |
| 2013-11-04 13:31 | yorik | File Added: notifylist.py | |
| 2013-11-06 19:53 |
|
Note Added: 0003862 | |
| 2013-11-07 14:57 | shoogen | Note Added: 0003867 | |
| 2016-11-07 21:50 | wmayer | Status | new => closed |
| 2016-11-07 21:50 | wmayer | Resolution | open => fixed |
