App.ActiveContainer

[DeepSOIC]

change behavior of ActiveDocument.addObject to default to document in case active container rejects the new object Here I am a bit confused again.

Modify Document::addObject() to handle exception when a container can't take the new object, and either go up the container tree till one that can accept is found, or (alternatively) proceed straight to adding directly to Document.

[DeepSOIC]

* change active container back to be tied to viewer (I realized it's a nice way to deal with Drawings). I am confused, but will deal with this point once I am done with the first three points

this is the trickiest part, and probably one of the main things to do. ActiveContainer signaling has to be redesigned, possibly needing a rewrite.

[DeepSOIC]

* fix docu string of Container, objects aren't available as attributes - I am a bit puzzled......

At some point, I made it possible to access a Sphere in a Part as Container(Part).Sphere. But then I have undone the change because of performance impact and potential for method-vs-docobject name clash. The possibility is still mentioned in python documentation string, I think, because I forgot to clean it out. See ContainerPy.xml:

Code: Select all

+Attributes: 
+Apart from built-in attributes and methods, child objects can be accessed as attributes of  
+Container object. E.g. 'Container(App.ActiveDocument).Part' is the same as  
+'App.ActiveDocument.Part', if Part is at the root of Document. 
+ 

[kcleung]

To be reworked.
* rename groupextension::allowobject
* fix docu string of Container, objects aren't available as attributes
* change behavior of ActiveDocument.addObject to default to document in case active container rejects the new object
* change active container back to be tied to viewer (I realized it's a nice way to deal with Drawings).

Do you really want to rename groupextension::allowobject? This will affect many other things too and be disruptive, so perhaps even if we want to do that, we may want to leave this as the last item.....

I will look at the rest of the TODO....

[DeepSOIC]

Do you really want to rename groupextension::allowobject? This will affect many other things too and be disruptive, so perhaps even if we want to do that, we may want to leave this as the last item.....

->

allow

The point is that virtual bool allowObject(DocumentObject* obj) of GroupExtension is a convenience function

It's not a "convenience" function.
allowObject(type, pytype): test if a new object of this type can be created in this container
allowObject(object): test if an existing object can be added to this container.
Different tests, can be different logic.

Maybe a rename is worth to emphasize these meanings.

+1

[ickby]

How does one get the list of immediate children of a Part? i.e. those contained in part, but not by any group inside? feels like a somewhat sophisticated search is required, similar to that needed to get immediate children of a Document.

You simply check if the object is part of a group. In python you can query getParentGroup() for that, in c++ use the static GroupExtension method for this. That search is exactly as sophisticated as before, just the other way around. Search parents instead of search children. (in my local branch with fixes to come the backlinks are enabled again, so no performance impact here)

I'm not happy about it. This design makes me want to curse

So I felt before the change, the design was aweful. But I think we have vastly different views here dependent on personal preference.

[kcleung]

How does one get the list of immediate children of a Part? i.e. those contained in part, but not by any group inside? feels like a somewhat sophisticated search is required, similar to that needed to get immediate children of a Document.

You simply check if the object is part of a group. In python you can query getParentGroup() for that, in c++ use the static GroupExtension method for this. That search is exactly as sophisticated as before, just the other way around. Search parents instead of search children. (in my local branch with fixes to come the backlinks are enabled again, so no performance impact here)

I'm not happy about it. This design makes me want to curse

So I felt before the change, the design was aweful. But I think we have vastly different views here dependent on personal preference.

I have just implemented the changes that makes Container::dynamicChildren() accurately get immediate children, and passes all the unit test. Also I have rebased the branch to the current master. Please have a look:

https://github.com/kcleung/FreeCAD/tree/ActiveContainer

[kcleung]

Do you really want to rename groupextension::allowobject? This will affect many other things too and be disruptive, so perhaps even if we want to do that, we may want to leave this as the last item.....

->

allow

The point is that virtual bool allowObject(DocumentObject* obj) of GroupExtension is a convenience function

It's not a "convenience" function.
allowObject(type, pytype): test if a new object of this type can be created in this container
allowObject(object): test if an existing object can be added to this container.
Different tests, can be different logic.

Maybe a rename is worth to emphasize these meanings.

+1

Ahhhhh I see..... you mean we should rename:

allowObject(type, pytype): test if a new object of this type can be created in this container

to something like allowObjectType(type, pytype) to emphasize its function, and

allowObject(object): test if an existing object can be added to this container.
should stay the same, right?


However, if we also rename allowObject(type, pytype) to allowObjectType(type, pytype) in other classes outside the Container folder such as GroupExtension, then I feel should be done by a senior developer (such as wmayer, ickby etc) who has access to the master branch the official repository, and should this refactoring be done on the master branch, we then just rebase our ActiveContainer branch accordingly. Also, this renaming will not affect the functionality of FreeCAD, so perhaps the priority will be much lower than our other TODO items.

[kcleung]

* fix docu string of Container, objects aren't available as attributes - I am a bit puzzled......

At some point, I made it possible to access a Sphere in a Part as Container(Part).Sphere. But then I have undone the change because of performance impact and potential for method-vs-docobject name clash. The possibility is still mentioned in python documentation string, I think, because I forgot to clean it out. See ContainerPy.xml:

Code: Select all

+Attributes: 
+Apart from built-in attributes and methods, child objects can be accessed as attributes of  
+Container object. E.g. 'Container(App.ActiveDocument).Part' is the same as  
+'App.ActiveDocument.Part', if Part is at the root of Document. 
+ 

Done. The python documentation is updated.

[kcleung]

Do you really want to rename groupextension::allowobject? This will affect many other things too and be disruptive, so perhaps even if we want to do that, we may want to leave this as the last item.....

->

allow

The point is that virtual bool allowObject(DocumentObject* obj) of GroupExtension is a convenience function

It's not a "convenience" function.
allowObject(type, pytype): test if a new object of this type can be created in this container
allowObject(object): test if an existing object can be added to this container.
Different tests, can be different logic.

Maybe a rename is worth to emphasize these meanings.

+1

Ahhhhh I see..... you mean we should rename:

allowObject(type, pytype): test if a new object of this type can be created in this container

to something like allowObjectType(type, pytype) to emphasize its function, and

allowObject(object): test if an existing object can be added to this container.
should stay the same, right?


However, if we also rename allowObject(type, pytype) to allowObjectType(type, pytype) in other classes outside the Container folder such as GroupExtension, then I feel should be done by a senior developer (such as wmayer, ickby etc) who has access to the master branch the official repository, and should this refactoring be done on the master branch, we then just rebase our ActiveContainer branch accordingly. Also, this renaming will not affect the functionality of FreeCAD, so perhaps the priority will be much lower than our other TODO items.

DeepSOIC: Do you agree with renaming allowObject(type, pytype) to allowObjectType(type, pytype) for FreeCAD.Containers.Container?

Wmayer and ickby: For the rest of the classes, such as GroupExtension, should we also rename allowObject(type, pytype) to allowObjectType(type, pytype) to emphasize that it is about the object type instead of the object, to distinguish from allowObject(object)?