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.
vocx
Posts: 1664
Joined: Thu Oct 18, 2018 9:18 pm

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

Postby vocx » Tue Aug 27, 2019 4:25 pm

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.
User avatar
yorik
Site Admin
Posts: 11565
Joined: Tue Feb 17, 2009 9:16 pm
Location: São Paulo, Brazil
Contact:

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

Postby yorik » 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.

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
Posts: 7052
Joined: Fri Aug 29, 2014 12:45 am
Location: Saint-Petersburg, Russia

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

Postby DeepSOIC » Wed Aug 28, 2019 3:17 pm

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
Posts: 1335
Joined: Fri Sep 16, 2016 9:28 pm

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

Postby mlampert » 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.
realthunder
Posts: 1204
Joined: Tue Jan 03, 2017 10:55 am

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

Postby realthunder » Mon Sep 02, 2019 3:04 am

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 (latest version 0.10.2) along 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
Posts: 1335
Joined: Fri Sep 16, 2016 9:28 pm

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

Postby mlampert » Mon Sep 02, 2019 5:34 am

realthunder wrote:
Mon Sep 02, 2019 3:04 am
There 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
Posts: 8440
Joined: Sun Sep 08, 2013 8:07 pm
Location: Zürich, Switzerland

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

Postby bernd » Mon Sep 02, 2019 12:44 pm

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.