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!
vocx
Veteran
Posts: 5197
Joined: Thu Oct 18, 2018 9:18 pm

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

Post by vocx »

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?
DeepSOIC wrote: Tue Aug 27, 2019 11:36 am 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),...
I think some tools from Arch also do the tests that DeepSOIC mentions, maybe Arch SectionPlane to extract the objects to cut and project.

In any case, my position is the same as furti's: it sounds as fundamentally bad design; however, if you already fixed this, and some objects don't provide a shape, then maybe it's a non-issue after all. We will have to keep testing and see if everything works as expected.
Always add the important information to your posts if you need help. Also see Tutorials and Video tutorials.
To support the documentation effort, and code development, your donation is appreciated: liberapay.com/FreeCAD.
User avatar
yorik
Founder
Posts: 13640
Joined: Tue Feb 17, 2009 9:16 pm
Location: Brussels
Contact:

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

Post by yorik »

In any case I would agree totally that if hasattr(obj,"Shape") is not a good test. Anybody could implement an object and choose to have any property or attribute named "Shape". We definitely need to change that to something better.

The more rock-solid test I used before was if obj.isDerivedFrom("Part::Feature"), which guarantees you that you have a Part-based object. But then, we began to have special objects like Arch BuildingPart (and I believe some in lattice2 too) which are not Parts (ie. we explicitly don't want them to be Parts) but still have the ability to hold a shape.

The best way would probably be to get the shape property by type, and not by name. But what if in the future we have objects with more than one shape (there might be already, doesn't the body have a tip shape too)?
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 »

yorik wrote: Wed Aug 28, 2019 2:35 pm In any case I would agree totally that if hasattr(obj,"Shape") is not a good test. Anybody could implement an object and choose to have any property or attribute named "Shape". We definitely need to change that to something better.
That's exactly what I would do to fool these tests intentionally, should I ever want to. IMO, there's nothing bad with this test: we test if an object has a shape; if it has that property, the one doing it definitely wanted his thing to have a shape, and so it's in our best interest to let the test pass, and cause an error later in code if the content of the Shape property is somehow invalid.
mlampert
Veteran
Posts: 1772
Joined: Fri Sep 16, 2016 9:28 pm

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

Post by mlampert »

What's the status on this? Is the null Shape property here to stay?

There are quite a few exceptions in Path due to this and I need to know how to proceed.
realthunder
Veteran
Posts: 2190
Joined: Tue Jan 03, 2017 10:55 am

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

Post by realthunder »

mlampert wrote: Mon Sep 02, 2019 3:01 am What's the status on this? Is the null Shape property here to stay?

There are quite a few exceptions in Path due to this and I need to know how to proceed.
There should be no null shape any more in upstream. Please report if otherwise.
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
mlampert
Veteran
Posts: 1772
Joined: Fri Sep 16, 2016 9:28 pm

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

Post by mlampert »

realthunder wrote: Mon Sep 02, 2019 3:04 amThere should be no null shape any more in upstream. Please report if otherwise.
My bad - I hadn't updated in a while. You are correct, after rebasing my changes the exceptions are gone.
Thanks!
User avatar
bernd
Veteran
Posts: 12849
Joined: Sun Sep 08, 2013 8:07 pm
Location: Zürich, Switzerland
Contact:

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

Post by bernd »

yorik wrote: Wed Aug 28, 2019 2:35 pm In any case I would agree totally that if hasattr(obj,"Shape") is not a good test. Anybody could implement an object and choose to have any property or attribute named "Shape". We definitely need to change that to something better.
here we are. The netgen mesh object has a property Shape which is AFAIK a property link to a Part object. https://github.com/FreeCAD/FreeCAD/blob ... ct.cpp#L82 I do not like it. For Gmsh I used another name. But since it has been there forever (forever in my FreeCAD lifetime not in Werners :mrgreen: ) I never changed it. https://github.com/FreeCAD/FreeCAD/comm ... c705f1eR59 I may should take into account to change it.
Post Reply