Why does App::MaterialObjectPython needs a Shape?

[vocx]

...
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?

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.

[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)?

[DeepSOIC]

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]

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]

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.

[mlampert]

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!

[bernd]

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 ) I never changed it. https://github.com/FreeCAD/FreeCAD/comm ... c705f1eR59 I may should take into account to change it.