Why does App::MaterialObjectPython needs a Shape?

Here's the place for discussion related to coding in FreeCAD, C++ or Python. Design, interfaces and structures.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
User avatar
furti
Posts: 344
Joined: Mon Nov 27, 2017 5:27 pm

Re: Why does App::MaterialObjectPython needs a Shape?

Post by furti »

realthunder wrote: Mon Aug 26, 2019 11:30 pm If there isn't, it uses Part.getShape() to obtain shape
But this is exactly the harmful thing. Wgen a simple group has no shape, it shouldn't have a shape property at all.
realthunder wrote: Mon Aug 26, 2019 11:30 pm so you can now simply use App.Link, App.Part, or even plain group to do modeling if you want, for Python code that is
But there is no need to have a Shape by default. When i want to turn a plain group into a shape, i can add a shape with the newly introducec dynamic properties beforehand. No need to "pollute" everything with a shape :)
realthunder
Veteran
Posts: 2190
Joined: Tue Jan 03, 2017 10:55 am

Re: Why does App::MaterialObjectPython needs a Shape?

Post by realthunder »

furti wrote: Tue Aug 27, 2019 5:27 am But there is no need to have a Shape by default. When i want to turn a plain group into a shape, i can add a shape with the newly introducec dynamic properties beforehand. No need to "pollute" everything with a shape :)
But you'll need logic to populate the shape, simply adding a dynamic property is not enough. The default 'Shape' is so that existing features, like Fuse, Cut, or other exotic Part Python features can work on these new types of objects without modification.

Any concrete example of having an extra Shape property breaking anything? I mean I have already modified the logic so that if Part.getShape() can't get anything, it won't provide the default 'Shape'. But if it can, will it break anything?
Try Assembly3 with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal
User avatar
furti
Posts: 344
Joined: Mon Nov 27, 2017 5:27 pm

Re: Why does App::MaterialObjectPython needs a Shape?

Post by furti »

realthunder wrote: Tue Aug 27, 2019 6:33 am Any concrete example of having an extra Shape property breaking anything?
The first post links to a help request where this caused a problem.

And given the huge amount of external workbenches there might be a lot of unspotted bugs because of this. Or maybe everything is fine. Nobody knows this.

But as DeepSOIC and yorik pointed out, they think they used the same check for shape properties too.

And despite the fact that this can cause problems, giving something a shape that is not an actual shape is not a good design decision in my opinion.

Where to stop when we once start with such things?
realthunder
Veteran
Posts: 2190
Joined: Tue Jan 03, 2017 10:55 am

Re: Why does App::MaterialObjectPython needs a Shape?

Post by realthunder »

furti wrote: Tue Aug 27, 2019 7:14 am The first post links to a help request where this caused a problem.
I have mentioned that I have already changed logic. In other word, the material object will no longer have a Shape.
And despite the fact that this can cause problems, giving something a shape that is not an actual shape is not a good design decision in my opinion.

Where to stop when we once start with such things?
Part.getShape() cannot just pull out some shape from nowhere. It currently support link and group. If for any reason you do actually want to differentiate real Shape property vs. Shape attribute, call obj.getPropertyByName('Shape'), and catch the AttributeError.
Try Assembly3 with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal
User avatar
furti
Posts: 344
Joined: Mon Nov 27, 2017 5:27 pm

Re: Why does App::MaterialObjectPython needs a Shape?

Post by furti »

realthunder wrote: Tue Aug 27, 2019 7:30 am I have mentioned that I have already changed logic. In other word, the material object will no longer have a Shape.
Oh sorry for that. I didn't understand your post like that :oops:

Will check this changes when available :)
User avatar
DeepSOIC
Veteran
Posts: 7896
Joined: Fri Aug 29, 2014 12:45 am
Location: used to be Saint-Petersburg, Russia

Re: Why does App::MaterialObjectPython needs a Shape?

Post by DeepSOIC »

realthunder wrote: Tue Aug 27, 2019 6:33 am Any concrete example of having an extra Shape property breaking anything? I mean I have already modified the logic so that if Part.getShape() can't get anything, it won't provide the default 'Shape'. But if it can, will it break anything?
From my perspective, not much. I have found only one place in my addons, where having a Shape attribute on Part object breaks something: MuxAssembly feature of Part-o-magic. which is essentially a replacement of that missing .Shape property, except it isn't affected by visibility, and has a few options such as flattening the resulting compound.

In there, the main use of hasattr(.., 'Shape') was to a) pick the objects to include in the compound, and b) tell apart Bodies (which totally have a useful Shape and should not be explored inside), from Parts, which didn't have a useful shape, and should be explored by the tool.
Fixing it shouldn't be a problem, I guess, as I can use something like 'Shape' in .PropertiesList instead. Or even leave it as is (but I'm not a fan of Part's Shape being affected by visibility of stuff inside; there should be another Visibility-like property for that, like child.Enable, or aPart.Tip)

Since my two addons are very shape-centric, having Shape property on Parts is generally a new feature for the workbench, not a problem.
realthunder
Veteran
Posts: 2190
Joined: Tue Jan 03, 2017 10:55 am

Re: Why does App::MaterialObjectPython needs a Shape?

Post by realthunder »

DeepSOIC wrote: Tue Aug 27, 2019 11:36 am I guess, as I can use something like 'Shape' in .PropertiesList instead.
Use getPropertyByName(), a lot faster.
there should be another Visibility-like property for that, like child.Enable, or aPart.Tip
Yes indeed. I'll implement that soon.
Try Assembly3 with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal
User avatar
DeepSOIC
Veteran
Posts: 7896
Joined: Fri Aug 29, 2014 12:45 am
Location: used to be Saint-Petersburg, Russia

Re: Why does App::MaterialObjectPython needs a Shape?

Post by DeepSOIC »

realthunder wrote: Tue Aug 27, 2019 12:04 pm Use getPropertyByName(), a lot faster.
:D

realthunder wrote: Tue Aug 27, 2019 12:04 pm Yes indeed. I'll implement that soon.
:D :D
User avatar
DeepSOIC
Veteran
Posts: 7896
Joined: Fri Aug 29, 2014 12:45 am
Location: used to be Saint-Petersburg, Russia

Re: Why does App::MaterialObjectPython needs a Shape?

Post by DeepSOIC »

Also, PoM Exporter and PoM ShapeGroup need patching because of this.
User avatar
DeepSOIC
Veteran
Posts: 7896
Joined: Fri Aug 29, 2014 12:45 am
Location: used to be Saint-Petersburg, Russia

Re: Why does App::MaterialObjectPython needs a Shape?

Post by DeepSOIC »

… PoM's ShapeGroup, Exporter and MuxAssembly should be working as before now, I have pathed them.
Post Reply