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