docs give examples of tuples where they should be Vectors

[marcus7070]

cadquery/cadquery/selectors.py

Lines 287 to 338 in e69b2f8

	class DirectionMinMaxSelector(Selector):
	"""
	Selects objects closest or farthest in the specified direction
	Used for faces, points, and edges
	Applicability:
	All object types. for a vertex, its point is used. for all other kinds
	of objects, the center of mass of the object is used.
	You can use the string shortcuts >(X|Y|Z) or <(X|Y|Z) if you want to
	select based on a cardinal direction.
	For example this::
	CQ(aCube).faces ( DirectionMinMaxSelector((0,0,1),True )
	Means to select the face having the center of mass farthest in the positive z direction,
	and is the same as:
	CQ(aCube).faces( ">Z" )
	"""
	def __init__(self, vector, directionMax=True, tolerance=0.0001):
	self.vector = vector
	self.max = max
	self.directionMax = directionMax
	self.TOLERANCE = tolerance
	def filter(self, objectList):
	def distance(tShape):
	return tShape.Center().dot(self.vector)
	# import OrderedDict
	from collections import OrderedDict
	# make and distance to object dict
	objectDict = {distance(el): el for el in objectList}
	# transform it into an ordered dict
	objectDict = OrderedDict(sorted(list(objectDict.items()),
	key=lambda x: x[0]))
	# find out the max/min distance
	if self.directionMax:
	d = list(objectDict.keys())[-1]
	else:
	d = list(objectDict.keys())[0]
	# return all objects at the max/min distance (within a tolerance)
	return [o for o in objectList if abs(d - distance(o)) < self.TOLERANCE]

The docs suggest DirectionMinMaxSelector((0, 0, 1), True), but this code:

from cadquery import *
test = Workplane("XY").box(1, 1, 1).faces(DirectionMinMaxSelector((0, 0, 1), True))

fails with AttributeError: 'tuple' object has no attribute 'wrapped'.

Changing the tuple to a Vector works:

test = Workplane("XY").box(1, 1, 1).faces(DirectionMinMaxSelector(Vector(0, 0, 1), True))

[jmwright]

Many of the methods will implicitly convert a tuple to a Vector so that either can be passed by the caller. Either we missed that in DirectionMinMaxSelector, or it was dropped in the transition to PythonOCC.

[jmwright]

Here's an example:

cadquery/cadquery/occ_impl/shapes.py

Line 330 in f9d9ae0

if type(basePointVector) == tuple:

[marcus7070]

Haha @jmwright, you're too quick, I was in the middle of writing the following when you commented.

Rather than change the docs I would suggest we consistently handle tuples or Vectors. But what is the consistent CadQuery way of doing this?

We check type for tuple here:

cadquery/cadquery/occ_impl/shapes.py

Lines 481 to 501 in e69b2f8

	def rotate(self, startVector, endVector, angleDegrees):
	"""
	Rotates a shape around an axis
	:param startVector: start point of rotation axis either a 3-tuple or a Vector
	:param endVector: end point of rotation axis, either a 3-tuple or a Vector
	:param angleDegrees: angle to rotate, in degrees
	:return: a copy of the shape, rotated
	"""
	if type(startVector) == tuple:
	startVector = Vector(startVector)
	if type(endVector) == tuple:
	endVector = Vector(endVector)
	T = gp_Trsf()
	T.SetRotation(gp_Ax1(startVector.toPnt(),
	(endVector - startVector).toDir()),
	angleDegrees * DEG2RAD)
	return self._apply_transform(T)

Request tuple in docs and init a Vector here:

cadquery/cadquery/selectors.py

Lines 62 to 94 in e69b2f8

	class NearestToPointSelector(Selector):
	"""
	Selects object nearest the provided point.
	If the object is a vertex or point, the distance
	is used. For other kinds of shapes, the center of mass
	is used to to compute which is closest.
	Applicability: All Types of Shapes
	Example::
	CQ(aCube).vertices(NearestToPointSelector((0,1,0))
	returns the vertex of the unit cube closest to the point x=0,y=1,z=0
	"""
	def __init__(self, pnt):
	self.pnt = pnt
	def filter(self, objectList):
	def dist(tShape):
	return tShape.Center().sub(Vector(*self.pnt)).Length
	# if tShape.ShapeType == 'Vertex':
	# return tShape.Point.sub(toVector(self.pnt)).Length
	# else:
	# return tShape.CenterOfMass.sub(toVector(self.pnt)).Length
	return [min(objectList, key=dist)]

But the Vector class handles being initiated with a Vector, so in the above both a Vector or a tuple would work.

Personally I want to check for the attribute the method needs. eg. DirectionMinMaxSelector needs to dot the vector, so change the init to

def __init__(self, vector, directionMax=True, tolerance=0.0001): 
         self.vector = vector if hasattr(vector, 'dot') else Vector(vector)

edit: NearestToPointSelectoractually fails with a vector, because it does Vector(*self.pnt).

[marcus7070]

Also found

cadquery/cadquery/cq.py

Lines 989 to 1013 in e69b2f8

	def transformed(self, rotate=(0, 0, 0), offset=(0, 0, 0)):
	"""
	Create a new workplane based on the current one.
	The origin of the new plane is located at the existing origin+offset vector, where offset is
	given in coordinates local to the current plane
	The new plane is rotated through the angles specified by the components of the rotation
	vector.
	:param rotate: 3-tuple of angles to rotate, in degrees relative to work plane coordinates
	:param offset: 3-tuple to offset the new plane, in local work plane coordinates
	:return: a new work plane, transformed as requested
	"""
	# old api accepted a vector, so we'll check for that.
	if rotate.__class__.__name__ == 'Vector':
	rotate = rotate.toTuple()
	if offset.__class__.__name__ == 'Vector':
	offset = offset.toTuple()
	p = self.plane.rotated(rotate)
	p.origin = self.plane.toWorldCoords(offset)
	ns = self.newObject([p.origin])
	ns.plane = p
	return ns

If you follow the code, Workplane.transformed -> convert to tuple -> Plane.rotated -> no check -> Plane.toWorldCoords -> convert to vector.

[jmwright]

@marcus7070 The inconsistent handling of tuples and Vectors is something that it would be nice to fix.

If a Vector can be constructed with either a tuple or a Vector, why add the hasattr checks? It seems like even our type(startVector) == tuple checks are redundant if Vector handles the conversion for us in each case. Is it to protect against the user passing another type of object, or is there another reason that I'm not seeing?

[marcus7070]

@jmwright Yeah, 99% redundant.

Vector init actually grabs an XYZ tuple from the argument and reconstructs a new Vector, so hasattr seems a bit more efficient to me. Also, on the off chance someone extended Vector through a subclass re-initing Vector would clobber it. But these are super minor issues, I would be happy with either method.

cadquery/cadquery/occ_impl/geom.py

Lines 11 to 51 in 3cd466a

	class Vector(object):
	"""Create a 3-dimensional vector
	:param args: a 3-d vector, with x-y-z parts.
	you can either provide:
	* nothing (in which case the null vector is return)
	* a gp_Vec
	* a vector ( in which case it is copied )
	* a 3-tuple
	* a 2-tuple (z assumed to be 0)
	* three float values: x, y, and z
	* two float values: x,y
	"""
	def __init__(self, *args):
	if len(args) == 3:
	fV = gp_Vec(*args)
	elif len(args) == 2:
	fV = gp_Vec(*args,0)
	elif len(args) == 1:
	if isinstance(args[0], Vector):
	fV = gp_Vec(args[0].wrapped.XYZ())
	elif isinstance(args[0], (tuple, list)):
	arg = args[0]
	if len(arg)==3:
	fV = gp_Vec(*arg)
	elif len(arg)==2:
	fV = gp_Vec(*arg,0)
	elif isinstance(args[0], (gp_Vec, gp_Pnt, gp_Dir)):
	fV = gp_Vec(args[0].XYZ())
	elif isinstance(args[0], gp_XYZ):
	fV = gp_Vec(args[0])
	else:
	raise TypeError("Expected three floats, OCC gp_, or 3-tuple")
	elif len(args) == 0:
	fV = gp_Vec(0, 0, 0)
	else:
	raise TypeError("Expected three floats, OCC gp_, or 3-tuple")
	self._wrapped = fV

[jmwright]

Ok, yeah. The hasattr check keeps from assuming too much. I'm ok with doing it that way.

[marcus7070]

Awesome, I might have a go at this when I get some time.

[marcus7070]

I've got an idea that I may as well document here, in case anyone else picks this up before I do.

Throughout CadQuery we accept tuples in the place of Vectors and type check or initialise Vectors in each function. There is a lot of code duplication and different implementations of this. It seems like a worthwhile task to centralise that code into one method we apply everywhere. IMHO a decorator makes sense for this job.

How should devs indicate that a particular argument represents a point in space and can be either a tuple or a Vector? Type hinting seems to make the most sense to me.

Here is a patch that implements it:

diff --git a/cadquery/cq.py b/cadquery/cq.py
index 25dc1d3..7577c4c 100644
--- a/cadquery/cq.py
+++ b/cadquery/cq.py
@@ -19,6 +19,9 @@
 
 import math
 from copy import copy
+from functools import wraps
+from inspect import signature
+import typing
 from . import (
     Vector,
     Plane,
@@ -33,6 +36,11 @@ from . import (
     exporters,
 )
 
+Numeric = typing.Union[int, float]
+Tuple2d = typing.Tuple[Numeric, Numeric]
+Tuple3d = typing.Tuple[Numeric, Numeric, Numeric]
+Pnt = typing.Union[Tuple2d, Tuple3d, Vector]
+
 
 class CQContext(object):
     """
@@ -54,6 +62,25 @@ class CQContext(object):
         self.tolerance = 0.0001  # user specified tolerance
 
 
+def tup_to_vec(f):
+    """
+    A decorator that looks for arguments type hinted as Pnts and if they are
+    not Vectors, converts them to Vectors. It is expected that any argument
+    that is not a Vector will be a tuple, but since this is Python we do not
+    type check.
+    """
+    sig = signature(f)
+    @wraps(f)
+    def wraps_f(*args, **kwargs):
+        bound_args = sig.bind(*args, **kwargs)
+        for name, param in sig.parameters.items():
+            val = bound_args.arguments[name]
+            if param.annotation == Pnt and not isinstance(val, Vector):
+                bound_args.arguments[name] = Vector(val)
+        return f(*bound_args.args, **bound_args.kwargs)
+    return wraps_f
+
+
 class CQ(object):
     """
     Provides enhanced functionality for a wrapped CAD primitive.
@@ -856,7 +883,8 @@ class CQ(object):
 
         return self.each(_rot, False)
 
-    def rotate(self, axisStartPoint, axisEndPoint, angleDegrees):
+    @tup_to_vec
+    def rotate(self, axisStartPoint: Pnt, axisEndPoint: Pnt, angleDegrees):
         """
         Returns a copy of all of the items on the stack rotated through and angle around the axis
         of rotation.
@@ -873,7 +901,8 @@ class CQ(object):
             [o.rotate(axisStartPoint, axisEndPoint, angleDegrees) for o in self.objects]
         )
 
-    def mirror(self, mirrorPlane="XY", basePointVector=(0, 0, 0)):
+    @tup_to_vec
+    def mirror(self, mirrorPlane="XY", basePointVector: Pnt = (0, 0, 0)):
         """
         Mirror a single CQ object. This operation is the same as in the FreeCAD PartWB's mirroring

I'm fairly confident I've defined those types incorrectly, but the code works.

Since that decorator inspects arguments, it would be pretty simple to add a try/except around the wrapped function and look for the common mistake of leaving the brackets off a tuple. If the Pnt argument and the one or two after it are all numeric types, append a suggestion to the error message that the user may have left the brackets off a tuple.

Anyhow, I haven't convinced myself that the extra dependencies (and adding partial type hinting) are worth it yet. I'm also not very experienced with type hinting. So I'm not going to make a draft PR yet, I'll just carry on with some other work and think about this for a while.

[marcus7070]

Seems like we want to add type hints to CadQuery anyway: #247