Plane class initialization
- Dominant language
- Python
- Stars
- 5.8k
- Forks
- 541
- Avg merge
- 3d 2h
- Merged PRs (30d)
- 5
Description
`Plane` class `__init__` method docstring says
:raises: ValueError if the specified xDir is not orthogonal to the provided normal.
This is not true (you can initialize a `Plane` with `xDir` not orthogonal to `normal`) nor necessary to define an orthogonal coordinate system (such a check would be in fact counterproductive). The only check should be on `xDir` not being parallel to `normal`, because `Plane` initialization performs a vector cross product between arguments `normal` and `xDir`, which raises the unintelligible error `Standard_ConstructionError: gp_Vec::Normalized() - vector has zero norm` if they are parallel. Try for example:
cq.Plane(origin=(0,0,0),xDir=(0,1,0),normal=(0,2,0))
The rationale is that, while the `normal` direction argument is invariant (that is, becomes directly the `zDir` of `Plane`), `xDir` direction changes if argument is not orthogonal to `normal`. In fact `xDir` argument is used (by means of cross products and "right hand" rule) just to find the actual `Plane` `xDir`, which must be orthogonal to zDir and yDir.
I believe docstring should be changed to
:raises: ValueError if the specified xDir is parallel to the provided normal.
and `_setPlaneDir` should be changed as below (or something similar)
def _setPlaneDir(self, xDir):
"""Set the vectors parallel to the plane, i.e. xDir and yDir"""
xDir = Vector(xDir).normalized()
self.yDir = self.zDir.cross(xDir)
if self.yDir.dot(self.yDir) < cls._eq_tolerance_dot: # is 'cls.' prefix really needed ?
raise ValueError("xDir should not be parallel to provided normal")
self.yDir = self.yDir.normalized()
self.xDir = self.yDir.cross(self.zDir)
This way `xDir`, `yDir` and `zDir` of a `Plane` are always mutually orthogonal, and the user, when defining a `Plane`, is not required to give redundant information, like an `xDir` argument strictly orthogonal to `normal` argument would be.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.